Skip to content
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

api: document the purpose of and review process for next.txt #32793

Open
bcmills opened this issue Jun 26, 2019 · 6 comments
Open

api: document the purpose of and review process for next.txt #32793

bcmills opened this issue Jun 26, 2019 · 6 comments
Assignees
Milestone

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 26, 2019

@andybons sent me CL 183919 and I wasn't quite sure what to do with it:

  • The diff from api/next.txt to api/go1.13.txt was not entirely trivial, but there were no visible failures on the build dashboard indicating any kind of skew (even for the symbols found in next.txt that were subsequently removed from the API).
  • The final update to go1.13.txt goes through @golang/osp-team as part of the release process, but it's not obvious to me what kinds of problems we're supposed to look for.
  • The last couple of CLs updating api/next.txt were sent between folks very central in the project, with very minimal descriptions and almost no visible discussion on the review thread.
  • Some recent CLs have updated api/next.txt in the same CL as the API addition itself, but it's not obvious to me which CLs should do the API update simultaneously vs. saving or leaving the update for someone else to review later.

api/README doesn't clarify these points very much. Its description of the file is:

next.txt is the only file intended to be mutated. It's a list of
features that may be added to the next version. It only affects
warning output from the go api tool.

We should expand the README file to more clearly describe the purpose and review process for next.txt.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 26, 2019

Updating the next.txt file affects the warnings that are printed at the end of an all.bash run.

The go1.13.txt file should be reviewed to make sure that all new symbols and all changes are intentional, and to make sure that they are all referenced, at least implicitly, by some release note.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 27, 2019

next.txt = "in all.bash please don't tell me about this anymore; we know this is new."
go1.13.txt = "we have committed to this as a stable API for go1.13"

The copying of lines from next.txt to go1.13.txt is meant to be accompanied by an actual audit that we want to commit to that API.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 27, 2019

Created #32813 for the audit of go1.13.txt.

@bcmills bcmills self-assigned this Jun 27, 2019
@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Aug 8, 2019

next.txt went empty for most of the Go 1.13 cycle, so I think it's clear the all.bash warnings are not very effective. Why not make them errors instead?

It would mean that a CL that adds new exported symbols also has to change next.txt at the same time, which seems like a good thing to me: it requires conscious acknowledgment of the new API surface.

/cc @dmitshur

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Aug 8, 2019

Failing all.bash on missing symbols from next.txt would also avoid the current scenario (https://golang.org/cl/189458) of missing symbols going unnoticed until the release. We'd still have to check that everything moved over from next.txt to go1.13.txt, but that's an audit step appropriate for release time.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 8, 2019

Changing next.txt in the same CL as the actual changes makes it less likely that revert CLs will apply cleanly.

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.