| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Two bits:
Sorry, something went wrong.
There was a problem hiding this comment.
@jasnell I chose exception because that's what it is called in the code. If we want to change it, do we have to reach into the code to change it there too?
I don't like error for the document because then you have to explain that error will be passed as an argument to any listeners on the 'error' event, which can get confusing especially to new users. So I'd probably go with reason over error, but reason doesn't signal to the reader that it's an error the way exception does.
As for the type, it would seem to be expecting an Error object but it actually doesn't look like there's any enforcement in the code so you could probably send it anything at all. I mean, it's just an argument for a listener, so it's kinda sorta up to the end user, I suppose. Suggest that it should (rather than must) be an Error perhaps?
Sorry, something went wrong.
|
@nodejs/documentation |
Sorry, something went wrong.
|
bump, especially on specifying the type. (See #7238 (comment) above. The code does not check for a type and any type could be useful in theory, at least. Or maybe Error makes the most sense, in practice?) |
Sorry, something went wrong.
|
Small doc improvement...review, anyone? @nodejs/collaborators |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
PR-URL: nodejs#7238 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
PR-URL: #7238 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
PR-URL: #7238 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
| Back | FazBrowse Home | New Git URL |
Checklist
Affected core subsystem(s)
doc net
Description of change