-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Modified pipePrefix to using relative path verses full path. #15988
Conversation
…ed because my full path was 120+ characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if CI is green
exports.PIPE = pipePrefix + pipeName; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit: this extra new line is unnecessary
CI will need to be re-run. It didn't run any tests/sub-tasks. Hopefully @nodjes/build has this fixed or will have it fixed soon... |
CI failures look unrelated. Landed in 971aad1 with #15988 (comment) fixed. Thanks for the contribution! |
Modified pipePrefix to use relative path on windows, previously tests failed when the full path was 120+ characters PR-URL: #15988 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Modified pipePrefix to use relative path on windows, previously tests failed when the full path was 120+ characters PR-URL: nodejs/node#15988 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Modified pipePrefix to use relative path on windows, previously tests failed when the full path was 120+ characters PR-URL: #15988 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Should this be backported to |
If this is backported it should come with #16364. |
Modified pipePrefix because if full path character count is to long you will get an error about port.
Test was done by using the following based on my directory ./node /Users/appleman/GitRepositories/github/NodeInteractiveWorkShops/FridayCodeAndLearn/node/test/parallel/test-tls-net-connect-prefer-path.js
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)