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

Jump to bottom

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
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Jun 26, 2019 •
edited

@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.
@bcmills bcmills added Documentation NeedsFix The path to resolution is known, but the work has not been done. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 26, 2019
@bcmills bcmills added this to the Go1.14 milestone Jun 26, 2019
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Jun 26, 2019
@ianlancetaylor
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

rsc commented Jun 27, 2019

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

@bcmills bcmills self-assigned this Jun 27, 2019
@FiloSottile
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rsc @FiloSottile @ianlancetaylor @bcmills @gopherbot