-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
errors,net: change internal/net.js to use internal/errors.js #11302
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,3 +86,6 @@ module.exports = exports = { | |
// Note: Please try to keep these in alphabetical order | ||
E('ERR_ASSERTION', (msg) => msg); | ||
// Add new errors from here... | ||
E('ERR_INVALID_PORT', (port) => { | ||
return `Port must be >= 0 and < 65536, got "${port}"`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the block is unnecessary, we can just
EDIT: no need for the quotes around There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer the quotes in the case it is not a number. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So should I keep the quotes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine with keeping the quotes. |
||
}); |
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.
I think it will be
#nodejs_error_codes
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.
https://github.com/nodejs/node/pull/11302/files#diff-63530ae59688fddc4c1638d1dfe88622R566
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.
@jasnell Yes, I realized it after a while. Shouldn't we let the tools take care of deciding the ids?
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.
I've done it this way to ensure that the ID's remain constant even if the automatic link generation changes or the structure of the document is refactored. I much prefer the explicit headers.
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.
Hmmm, I am not sure. We lose consistency here.