-
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: cleanup/update test-dgram-error-message-address.js #8938
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 there is some other cleanup that you could do while you're here. Like getting rid of assert.equal()
and assert.notEqual()
.
These changes would make this change more compelling:
|
Fixes: #8925 |
@Goyapa while you are at it can you please replace Also you can use the |
@ipinca |
@Goyapa we can use https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/ if it is not a problem for you. |
You can also try to edit files on your branch using Github’s web interface, it’s a bit tricky but it’s nice when you don’t want to or can’t use a local git client. |
@lpinca |
c133999
to
83c7a88
Compare
@lpinca |
* Changes: * var -> const * assert.equal() -> assert.strictEqual() * assert.notEqual() -> assert.notStrictEqual() * Fixes: #8925
Landed in 6a73d61 |
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
dgram
Description of change
test: cleanup/update test-dgram-error-message-address.js