Repository navigation
Confusion around semver-minor commits #820
Description
Activity
I can't add the agenda label, but I appreciate if someone can add it.
That's usually up to the releaser to decide. If there's no
notable-changelabel a semver-minor might or might not be added as a notable change, except in the case of major releases https://github.com/nodejs/node/blob/main/doc/contributing/releases.md#generate-the-notable-changes.Sometimes we forget and sometimes is just hard to define if a commit is notable or not without having the label explicitly in the PR. Therefore, I suggest if you feel your PR is notable, add the label.
Reacted by Ruy AdornoIdeally all semver-minors would be considered notable by default, unless it was a special case that wasn't really notable.
Could the label instead be "this minor change is NOT notable", so that minor changes are included by default?
Reacted by Michaël Zasso, Beth Griggs and Richard LauIdeally all semver-minors would be considered notable by default, unless it was a special case that wasn't really notable.
I agree with that, and
git node release --prepare(which I always use) generates a changelog based on this assumption.Reacted by Jordan HarbandThis was discussed in the v18.14.0 proposal yesterday (nodejs/node#46396 (comment))
@tniessen, generally we've defaulted to elevating all semver minors to notable, but only demote them in the cases we're certain they really are not useful to highlight to end-users.
This was the way I (and I suspect others) were advised to craft notable changes during the release onboarding process. It's alluded to in releases.md - but our release automation also currently makes the same assumption when generating the notable changes (nodejs/node-core-utils/blob/main/lib/prepare_release.js).
I think it is a fair order of operations to consider non-notable semver minors the exception - especially considering releasers do not always have the full context of the commits.
Today, I realized that the latest v18 release did not include semver minor changes in the notable changes section. (https://github.com/nodejs/node/releases/tag/v18.14.0)
For v18.14.0 @juanarbol was asked to mark some minors as not notable in both nodejs/node#46396 (comment), nodejs/node#46396 (review).
It seems nodejs/node#45947 is the only minor that wasn't explicitly asked to be demoted. It seems to have got lost somehow - it wasn't mentioned this list (nodejs/node#46396 (review)). If you prefer nodejs/node#45947 to be marked as notable we can retroactively update the changelog and website blogpost (we have done so in the past).
Could the label instead be "this minor change is NOT notable", so that minor changes are included by default?
The label is not specific to semver minors... it can also be applied to semver patch PRs if the bug being fixed is significant.
Reacted by Jordan HarbandThat's usually up to the releaser to decide.
@RafaelGSS I understand that it's up to the releaser, but I think releasers should not make conflicting decisions about the same pull request on different releases. The sentence in
Use your judgment there. I strongly believe that we should remove and/or change this sentence from the release contributing document:Some SEMVER-MINOR commits may be listed as notable changes on a case-by-case basis. Use your judgment there.Ideally all semver-minors would be considered notable by default, unless it was a special case that wasn't really notable.
@ljharb This is not mentioned in the release document. The document states that
Some SEMVER-MINOR commits may be listed as notable changes on a case-by-case basis. Use your judgment there., and this is why it is causing conflicts.If you prefer nodejs/node#45947 to be marked as notable, we can retroactively update the changelog and website blogpost (we have done so in the past).
@BethGriggs Thank you for the offer. It would be unfair to other semver-minor commits, which should be treated as same as the referring commit. So, I don't think we should do it.
I think it would be beneficial to have a written decision/rule about this so that there won't be any confusion. Due to the nature of the label
notable, it is and will create emotional conflicts for collaborators. On top of that, as a non-native speaker, the wordnotableplays an important part in these kinds of conflicts. I strongly believe that it sends a wrong message (and demotivates) when a label is markednotable-changeand later removed by a TSC member because the requirements for anotable-changeis not clearly defined in the release guidelines.PS: I don't have a strong opinion towards the decision, but it does not feel right to me to see the same commit marked as a notable change in v19 and not in v18.
@anonrig that makes sense - i'm not familiar with the release document. my comment was aspirational tho, so i'd be happy to see the document updated :-) in particular, it's nice to not have to look through the commit/PR list in order to figure out what's added.
Releaser's approach (and as what is implemented by our tooling today) was clarified in nodejs/node#46592:
By default, the
### Notable changessection should be populated with the
commits in the release that have either thenotable-changeorsemver-minor
label. Somesemver-minorfeatures may be determined by the releaser, or
indicated by another contributor, to not be appropriate to be listed as a
notable. The ultimate decision rests with the releaser.Thanks for the update @BethGriggs. I'm closing this issue for now.
Reacted by Beth Griggs
In the latest v19, there was a discussion around whether all semver minors are considered as a notable change, and later a semver minor change was added as a notable change(example: adding buffer.isAscii)
Today, I realized that the latest v18 release did not include semver minor changes in the notable changes section. (https://github.com/nodejs/node/releases/tag/v18.14.0)
What is the procedure about this? Collaborators have a conflict about this issue, and it would help a lot if the releasers solve this conflict.
Thank you!