-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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: update test to use common.fail #9900
test: update test to use common.fail #9900
Conversation
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, but the commit message is only partially accurate.
What could i add to the commit message to make it more accurate? |
Most of the body is fine. It's just the first line. Maybe just |
@jaybrownlee You should format the commit message as described in CONTRIBUTING guidelines. |
use common.fail instead of assert(false) change var to let or const as appropriate
79bb484
to
8dc87b6
Compare
commit message updated as suggested |
@jaybrownlee Thank you. Looks good 👍 |
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: #9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in fa22854. Thank you for the PR and for participating in the code-and-learn! |
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: #9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: nodejs#9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: nodejs#9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: #9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: #9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
use common.fail instead of assert(false) change var to let or const as appropriate PR-URL: #9900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
Description of change
use common.fail instead of assert(false)
also changes var to let & const as appropriate