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: assert path.join() arguments equally #2159

Closed
wants to merge 1 commit into from

Conversation

phillipj
Copy link
Member

Re-use assertPath() when asserting path argument types in join() as throughout the rest of the path module.

This also ensures the same error message generated for posix as for win32.

Re-use `assertPath()` when asserting path
argument types in `join()` as throughout
the rest of the `path` module.

This also ensures the same error message
generated for posix as for win32.

PR-URL: nodejs#2159
@mscdex mscdex added the path Issues and PRs related to the path subsystem. label Jul 10, 2015
@silverwind
Copy link
Contributor

LGTM

@thefourtheye
Copy link
Contributor

LGTM. Hope we are not breaking any tests.

@phillipj
Copy link
Member Author

All tests pass :) The existing tests ensures there's an TypeError thrown on invalid input, no assertions on the message itself.

@thefourtheye
Copy link
Contributor

@phillipj Even I tried to change this here ;-)

@phillipj
Copy link
Member Author

Hah, lets hope it lands this time then @thefourtheye :)

silverwind pushed a commit that referenced this pull request Jul 11, 2015
Re-use `assertPath()` when asserting path argument types in `join()`
as throughout the rest of the `path` module.

This also ensures the same error message generated for posix as for
win32.

PR-URL: #2159
Reviewed-By: Roman Reiss <me@silverwind.io>
@silverwind
Copy link
Contributor

Landed in 2ba8460

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants