-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
are the YAML "added" annotations ambiguous? and are they rewritten on backport? #24850
Comments
@nodejs/docker |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The problem is not exactly with the release process. When the PR lands on master, it should already include a REPLACEME placeholder in the docs metadata. Then when releasers include the change, that placeholder is replaced with the actual version number. The release commit that does it is cherry-picked to master. |
Sorry at-nodejs/docker! I meant @nodejs/documentation . |
The main issue is that we forget too often to include the placeholder in semver minor changes |
Yeah, I would assume that what releasers have done so far when cherry-picking the release commit to master from an older (e.g. LTS) release line, is to solve the merge conflict with the existing |
@targos that happens when a change is released in a "current" release branch, but what about when it is released further, into the LTS branches? |
@sam-github I have never prepared an LTS release so I'm not sure but this part of the process should be identical. |
@addaleax the process you describe seems like it might work... any reason why it didn't for URL? #24847 predates that process? The process is not documented? The process wasn't followed? @targos that part of the process can't be identical, can it? When picking from master to current, there are REPLACEME tags in the markdown files. When picking from current to LTS, there will never be REPLACEME tags in the markdown files, I don't think, since those tags were removed in the initial release. Do I misunderstand? |
Anyway, one thing that we currently don't do, is cherry-picking between release lines. So in the current state, if no mistake was done and a change happened in v11.4.0 and later in v10.15.0, only the documentation on master would have both "added" fields. Release docs would only include their corresponding version |
Is it what LTS releasers do? In that case you're right |
Actually, maybe it isn't, because the REPLACEME does seem to get updated on LTS branches. I don't have the time to do a full audit, but the first SEMVER-MINOR in CHANGELOG_V10.md is servers.headerTimeout, here's the situation with it, probably its typical:
The docs look to me like they contain info only for their release lines, unless someone manually, like @watson in #24847, manually updated them, which is pretty onerous. Also, there aren't any docs on the meaning of the tags, so the current situation is not obvious. Sometimes the docs do have info about old release lines, so anybody reading the docs could justifiably assume that when the info is missing, its because the feature wasn't backported. |
@nodejs/lts ----^ |
I think this was fixed in #33042 |
cf. #24847
The added annotations are very useful, and at the same time, I think they are slightly ambiguous, or at least their interpretation is subtle! And undocumented: https://nodejs.org/api/documentation.html at least doesn't describe them, but perhaps they are described elsewhere?
Examples:
Is the API present in 8.0.0? I think it would have to be, because if it was added in
>8.0.0
and backported then this would look like:Correct? But a bit subtle!
Its not clear to me whether @nodejs/releasers have a process to update the YAML tags when backporting/cherry-picking. Because #24847 I suspect not. It would be kind of onerous, since both the backported branch and the master needs to be updated to reflect the release of the API on the old branch. I don't know how much energy should be spent in keeping the annotations up to date on the release branches, but it would be great if at least the master & current documentation was complete.... meaning it would need updating after every release. :-(
Summary: I think that ideally:
The text was updated successfully, but these errors were encountered: