-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
test: remove unnecessary status check on test-release-npm #47516
Conversation
Signed-off-by: RafaelGSS <[email protected]>
cc @nodejs/npm |
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.
Agreed that in light of #43716 this is an unnecessary check.
Fast-track has been requested by @RafaelGSS. Please 👍 to approve. |
I'm requesting |
The GitHub UI shows a pending CI, but the CI link shows it as finished. Should I land it manually? @nodejs/build |
Yes, if you are reasonably certain that the GitHub widget is showing bad information and CI is in fact complete and successful, you can always land manually. |
Landed in e67b944 |
Signed-off-by: RafaelGSS <[email protected]> PR-URL: #47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: #47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: #47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: #47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: #47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: nodejs#47516 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
test-release-npm
is failing on #47381 and it seems related to #43716. However, this asserting seems unnecesary, so rather than adjust it (either onnpm
side or node.js) sounds reasonable to remove that check.However, it doesn't mean there's no bug in that change. It seems
process.exitCode
isn't considered whenprocess.exit()
is called withundefined
. cc: @daeyeon