doc: error message are still major#14375
Uh oh!
There was an error while loading. Please reload this page.
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?
refack commented Jul 20, 2017
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.
refack commented Jul 20, 2017
To all who approved, current status is that the sentence dealing with future plans was dropped. PTAL. |
Trott commented Jul 20, 2017
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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
refackforce-pushed the semver-major-error-still branch from fa0fccb to ac81267Compare
PR-URL: #14375 Refs: #13937 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #14375 Refs: #13937 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
13 participants
Ratifying the decision from #13937
Checklist
Affected core subsystem(s)
doc,error