Better commit message for breaking changes. #16846
Conversation
|
For posterity, this is heavily influenced by my lost attempt at making a Node 7 to 8 breaking changes doc (https://github.com/nodejs/node/wiki/Breaking-changes-between-Node-7-and-8). The changes are too many and too obscure for a single person (or likely, even a small group of people) to do at major releases. (Certainly, I didn't even bother for Node 9, which what, probably has even more changes??) The purpose of such a doc is to allow people with fairly low knowledge of Node.js internals to look for something they are using and identify what might have changed in it's use.
An actual formatted item that could be directly inserted into the doc would be ideal, but even that info might've made my job feasible. |
Add informations to CONTRIBUTING.md requesting the committers and especially the one working on breaking changes to write proper commits messages containing the reason behind the breaking changes, the case during which it can be triggered, and a description of the breaking change in itself.
Fishrock123
added
the
meta
CONTRIBUTING.md
Outdated
| @@ -334,6 +334,10 @@ use `Refs:`. | |||
| - `Refs: http://eslint.org/docs/rules/space-in-parens.html` | |||
| - `Refs: https://github.com/nodejs/node/pull/3615` | |||
|
|
|||
| 5. If your commit introduce a breaking change (`semver-major`), it should contain | |||
CONTRIBUTING.md
Outdated
| 5. If your commit introduce a breaking change (`semver-major`), it should contain | ||
| an explanation about the reason of the breaking change, which situation would trigger | ||
| the breaking change and what is the exact change. | ||
|
|
There was a problem hiding this comment.
How about adding something like this:
Breaking changes will be listed in the wiki with the aim to make upgrading easier. Please have a look at Breaking Changes for the level of detail that's suitable.
There was a problem hiding this comment.
Sounds good to me. I will make a follow-up commit
- Fix a grammar mispelling - Add a paragraph describing the use and the ideal commit messages.
|
@Fishrock123 Can you give examples of PRs that don't already do that? Breaking changes get the semver-major tag and if there's one thing I've been hammering on all these years, it's that authors should write up good commit logs. |
|
@Fishrock123 It could be a good occasion to propose a parsable format for For example adding something like this after the description: |
|
I can add this onto this PR if you want (which seems to be a clear gain for me, the ability to grep through the commit label could be kind of a game changer for CLI addict and automation) |
|
Don't remember any examples, sorry. You'd have to look through it yourself. |
|
Oh and uh, I don't think commit metadata is useful here. Proper notes in the PR should work fine. |
|
@Fishrock123 Isn't this maintaining a dependency to Github ? |
|
@MatonAnthony Thanks so much for your first commit 🎉. I landed this as |
Add informations to CONTRIBUTING.md requesting the committers and especially the one working on breaking changes to write proper commits messages containing the reason behind the breaking changes, the case during which it can be triggered, and a description of the breaking change in itself. PR-URL: #16846 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
|
@MatonAnthony It is, but we already have that dependency (tags, discussions) so... yeah. |
|
We should find a way to broadcast important changes, such as this. |
|
@refack Yes, what do you suggest? |
|
Should we have some kind of format for those descriptions? Maybe a bullet list following a common sentence like "this patch introduces breaking changes:"? |
ping @nodejs/collaborators is a start (I was also thinking of a low-traffic mailing list,
Suggestion: where |
|
One thing to think about is sometimes its not known that a commit is broken until after it has landed, and sometimes we go back and forth about semver-majorness after the fact, so there is some advantage to leaving that info as mutable metadata aka github labels. But I agree that the fact that its not clear in the commit message is unfortunate. |
Add informations to CONTRIBUTING.md requesting the committers and especially the one working on breaking changes to write proper commits messages containing the reason behind the breaking changes, the case during which it can be triggered, and a description of the breaking change in itself. PR-URL: #16846 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
Add informations to CONTRIBUTING.md requesting the committers and especially the one working on breaking changes to write proper commits messages containing the reason behind the breaking changes, the case during which it can be triggered, and a description of the breaking change in itself. PR-URL: #16846 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
In this change, I would like to propose to emphasize on the importance of a good commit message in general.
But also and especially on breaking changes to make the documentation process easier.
I think the readiness of the commit message and a proper and detailled content can make a whole world of difference to understand breaking changes and the reason behind them.
This suggestion comes from a Twitter discussion available here: https://twitter.com/MylesBorins/status/927625601112006656
Checklist
Affected core subsystem(s)
PS: I am sorry, I'm proposing a change regarding commit messages and mine is awful, I did this on Github editor, I will rebase this commit to change its message at once.