go icon indicating copy to clipboard operation
go copied to clipboard

x/build/cmd/relnote: missed API changes

Open rsc opened this issue 2 years ago • 6 comments

Somehow we missed the addition of binary.NativeEndian, which is listed in the API files, when preparing the Go 1.21 documentation. The existence in the API files marked with #57237 should have provoked relnote to insist on a reference to go.dev/issue/57237 in go1.21.html, but there is no such mention. #62349 tracks updating the docs. This issue is about fixing the process that led to the incomplete docs.

Just before the release we noticed that we'd forgotten to document the new runtime.PanicNilError, and again it seems like relnote should have noticed and caught that for us.

Marking a release blocker for Go 1.22 because we need to understand why we are dropping release notes on the floor. Either we are not using relnote often enough or correctly or there is a bug in it.

rsc avatar Aug 30 '23 15:08 rsc

Looking at cmd/relnote, I think it's this line:

				if issue := gh.Issue(num); issue != nil && !issue.ClosedAt.Before(cutoff) && hasLabel(issue, "Proposal-Accepted") {

Note `ClosedAt. The issue in question, #57237 was not closed. I think that it wasn't closed because there were two parts to it, one for encoding/binary and one for x/sys/cpu. The two independent CLs referenced it but didn't close it. I just closed it today as I think the work is complete.

ianlancetaylor avatar Aug 31 '23 00:08 ianlancetaylor

I spotted another source of possible inaccuracies in relnote while looking into this earlier: it uses a fairly crude heuristic for computing the start and end of each major Go release cycle, exactly 6 months apart (source). That heuristic is still fairly accurate, but the development cycle is now slightly expanded due to recent changes including a late freeze and early thaw, which may also contribute to some items being missed if they were at the very start or end of a cycle.

@jba Are you interested in taking this issue since you're working on #64169 now and this seems related?

dmitshur avatar Nov 15 '23 16:11 dmitshur

Yes, I'll own it.

jba avatar Nov 15 '23 18:11 jba

Change https://go.dev/cl/542855 mentions this issue: cmd/relnote: warn about unclosed proposals with CLs

gopherbot avatar Nov 16 '23 12:11 gopherbot

Marking this as a release blocker for Go 1.23 because the general problem of missing API changes may still remain. I don't understand how relnote can work on the new markdown files in the RELNOTE=yes CL comment case, since there are no CL numbers in the new markdown files for it to look for. Or are there?

rsc avatar Apr 22 '24 16:04 rsc

We shouldn't miss API changes, because there is a test in the main repo. We may miss other things, like changes to commands.

RELNOTE= comments are obsolete in the current design; authors should instead add a doc/next/*.md file, possibly with a TODO. The relnote todo command would print these out.

This wasn't communicated clearly in the README; fixed in https://go.dev/cl/582075.

The relnote todo will look for these, since they may keep happening (https://go.dev/cl/582097). As you say, we can't point to places in the markdown files that need to be modified, but at least we won't drop anything.

We should probably also ask authors of release notes about accepted proposals to put the issue number in a comment in the note. Then we can look for CLs that mention accepted proposals that don't appear in the notes. That is future work.

jba avatar Apr 26 '24 21:04 jba

Change https://go.dev/cl/582097 mentions this issue: cmd/relnote: improve todo subcommand

gopherbot avatar Apr 29 '24 12:04 gopherbot

@jba @rsc is there anything else that needs to be done for this? Thanks.

cherrymui avatar May 08 '24 16:05 cherrymui

This:

We should probably also ask authors of release notes about accepted proposals to put the issue number in a comment in the note. Then we can look for CLs that mention accepted proposals that don't appear in the notes.

For this cycle, we can at least look for CLs that mention accepted proposals. Then the release team can manually verify that they have release notes. I will improve relnote todo to do that, probably by the end of the week.

jba avatar May 08 '24 17:05 jba

Note that https://go-review.googlesource.com/c/build/+/410244 already added "look for CLs that mention accepted proposals", but it seems to have been disabled. Re-enabling it would be great.

rsc avatar May 09 '24 14:05 rsc

Also:

RELNOTE= comments are obsolete in the current design; authors should instead add a doc/next/*.md file, possibly with a TODO. The relnote todo command would print these out.

It's not reasonable to expect everyone to be up to speed on the new process. If people write RELNOTE= comments, we should still pick them up instead of blaming people for not knowing the process changed.

(RELNOTE=/RELNOTES= is an API, and we shouldn't make a breaking change here. It is fine to add new API, just not fine to delete old API.)

rsc avatar May 09 '24 14:05 rsc

I also sent CL https://go-review.googlesource.com/c/build/+/584303 to restore the cutoff date calculation.

rsc avatar May 09 '24 14:05 rsc

I haven't written release notes yet for the synctimer proposal. I'd like to see relnote catch that before I do.

rsc avatar May 09 '24 14:05 rsc

I found https://go-review.googlesource.com/c/build/+/582097 which says:

The cutoff date must now be provided manually on the command line, avoiding errors in calculating it automatically.

This seems completely backward to me. If the automated calculation has a bug, then that's all the more reason to fix the bug instead of falling back to buggy humans doing work that a computer can do. I should be able to run relnote todo to check what I might need to do without having to get out a calendar.

If calendrical calculations aren't good enough, the code can look for release tags in the maintner logs. It clearly has all the information it could possibly need.

rsc avatar May 09 '24 14:05 rsc

Apologies for the issue comment storm. I seem to have jumped into the middle of work on relnote and it took me a few messages to get up to speed. I see now also that the RELNOTE= comments are processed again.

If we can get to the point where relnote todo (with no arguments) will tell me what release notes still need to be written, I know I have at least two proposal-related ones (synctimer and godebug). But I'd like to see the tool tell me what else I am missing too.

rsc avatar May 09 '24 15:05 rsc

I just landed some CLs that I think do what you're asking. They will print a TODO for any accepted proposal mentioned by a CL merged after the cutoff. The old relnote cared about when the proposal issue was closed, but the new code does not: I think that any work done for a proposal, whether that proposal was closed years ago or is still open, is potentially worthy of a release note.

jba avatar May 09 '24 18:05 jba

If the automated calculation has a bug, then that's all the more reason to fix the bug instead of falling back to buggy humans doing work that a computer can do.

I'll take that on. I was using the first line of https://go.dev/doc/devel/release#go1.22.0. @dmitshur Where can I get that information in a more machine-readable form?

jba avatar May 09 '24 18:05 jba

Thanks. That info is in x/website/internal/history, but you can't import it. You can do the https fetch to go.dev though. It's template-generated so any simple regexp you use is likely to keep working.

rsc avatar May 09 '24 19:05 rsc

Or you could look in master for the git commit that tags the release and use that date. But I thought the problem was that the dev cycle might have started before the release?

There's not going to be one exact date that is right necessarily. It's better to be on the early side than the late side, since early means false positives, and those can be silenced, but false negatives are never noticed. The way we used to deal with false positives is to add a comment to the html file mentioning the CL or issue, so that the tool would see that it was 'handled'. Something like that probably works here too.

rsc avatar May 09 '24 19:05 rsc

I agree that just using the release date of the .0 release isn't likely to be very useful. The updated release cycle, tree reopening is nominally scheduled for third week of July or January (see go.dev/s/release#…-work-on-the-next-release-begins). We have generally hit that target, but I don't think it's possible to guarantee it.

To add to Russ' comment above, maybe you can deal with false positives from looking a bit too far back by checking the release notes for the prior release. But you'd need to fetch them either from the x/website repo, or from go.dev itself, which has overhead.

There are probably other possible approaches. For instance, each development cycle normally begins with a CL that updates the constant in the internal/goversion package (e.g., CL 557155 did it for #64340 to reopen the tree for 1.23 development; this is tracked in #40705), maybe that's something to take advantage of.

dmitshur avatar May 09 '24 19:05 dmitshur

This is something we can just fix once at the start of each cycle by manually overriding the few false positivies. The cutoff date is also something we can hard-code once per cycle in the tool, or we could even read it out of the md files.

rsc avatar May 09 '24 19:05 rsc

@jba Checking on from a weekly release meeting. Is there more to do before considering this resolved? Thanks.

dmitshur avatar May 15 '24 16:05 dmitshur

Not as far as I know.

jba avatar May 15 '24 21:05 jba