doc: error message are still major#14375
Conversation
refack
added
errors
doc/guides/using-internal-errors.md
Outdated
There was a problem hiding this comment.
typos: than, and commas after future and )
also, I’d prefer Node 10.x over v10
There was a problem hiding this comment.
I don't believe we can say "no sooner" than v10. The decision was to handle the changes as semver-major in 8.x and to revisit within 9.x... which means we could start handling them as semver-minor as early as 9.0.0.
doc/guides/using-internal-errors.md
Outdated
doc/guides/using-internal-errors.md
Outdated
There was a problem hiding this comment.
will hopefully? (if we're optimistic)might? (if we're skeptic)- or even
error message changes will be reevaluated, with the goal being to handle then as(if we want to explain)
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
Left some nits, but with or without 'em, LGTM.
doc/guides/using-internal-errors.md
Outdated
There was a problem hiding this comment.
Still bikeshedding over will: #14375 (review)
There was a problem hiding this comment.
what about should? That gives us the opportunity to change if we absolutely have to, but shows that is the direction we are trying to go?
|
Went with In the future, after the transition has been completed, error message changes should be handled
as `semver-minor` or `semver-patch`. |
doc/guides/using-internal-errors.md
Outdated
There was a problem hiding this comment.
Nit: This makes it sound like once the errors have been all migrated, the change to treating them as not-breaking is automatic. Honestly, I think the best/easiest thing to do at this point is to remove the sentence. It's not needed. It's about a possible future state that is likely to come (but not guaranteed) and we certainly haven't agreed as to exactly when it will come. So just leave it out.
There was a problem hiding this comment.
Makes sense.
Originally I thought we should end with a "hopeful" note. But that's sort of the subject of the guide as a whole.
|
To all who approved, current status is that the sentence dealing with future plans was dropped. PTAL. |
|
Still LGTM. |
doc/guides/using-internal-errors.md
Outdated
There was a problem hiding this comment.
I don't think this is 100% true, if the error message remains the same?
There was a problem hiding this comment.
The Error's name changes to ${super.name} [${this[kCode]}] so the .toString() changes.
PR-URL: nodejs#14375 Refs: nodejs#13937 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
refack
force-pushed
the
semver-major-error-still
branch
from
fa0fccb to
ac81267
Compare
PR-URL: #14375 Refs: #13937 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: #14375 Refs: #13937 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
13 participants
Ratifying the decision from #13937
Checklist
Affected core subsystem(s)
doc,error