Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

path: reduce type checking on some methods #1216

Closed
wants to merge 1 commit into from
Closed

path: reduce type checking on some methods #1216

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 20, 2015

a465840 added strict type checking for the methods in the path module. However, dirname(), basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those methods. Fixes #1215.

R=@rvagg

a465840 added strict type
checking for the methods in the path module. However, dirname(),
basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those
methods.
@brendanashworth
Copy link
Contributor

Should a deprecation message be added for these functions?

@rvagg
Copy link
Member

rvagg commented Mar 20, 2015

for 1.6.1 this lgtm, we could improve later

cjihrig added a commit that referenced this pull request Mar 20, 2015
a465840 added strict type
checking for the methods in the path module. However, dirname(),
basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those
methods.

Fixes: #1215
PR-URL: #1216
Reviewed-By: Rod Vagg <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 20, 2015

Landed in 8de78e4.

@cjihrig cjihrig closed this Mar 20, 2015
@cjihrig cjihrig deleted the 1215 branch March 20, 2015 01:12
@rvagg rvagg mentioned this pull request Mar 20, 2015
rvagg added a commit that referenced this pull request Mar 20, 2015
Notable Changes:

* path: New type-checking on path.resolve()
  <#1153> uncovered some edge-cases
  being relied upon in the wild, most notably path.dirname(undefined).
  Type-checking has been loosened for path.dirname(), path.basename(),
  and path.extname(), (Colin Ihrig)
  <#1216>.
* querystring: Internal optimizations in querystring.parse() and
  querystring.stringify() <#847>
  prevented Number literals from being properly converted via
  querystring.escape() <#1208>,
  exposing a blind-spot in the test suite. The bug and the tests have
  now been fixed (Jeremiah Senkpiel)
  <#1213>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants