-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Keep design documents current #5606
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
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I've also changed the TODO's to be blockquotes, for increased visibility.
proposals/p5606.md
Outdated
proposal PR to the corresponding rendered document is much more involved. There | ||
is a rendered markdown preview accessible from GitHub's PR UI, but that has | ||
several drawbacks: | ||
|
||
- It's modeled as transient browser state, so it disappears on reload, and | ||
links to it can't be usefully shared. | ||
- It contains no internal section link anchors, so the table of contents is | ||
broken, as are any other internal links in the document. | ||
- It's styled with inconsistent and distracting diff-marker highlights. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is maybe overlooking ...
-> View file
which takes you to a proper URL with properly formatted markdown.
This gives the version of the file in the PR, rather than at trunk FWIW. I think that is the primary difference between links to the PR and to the file path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I was indeed overlooking that. Today I learned!
I've revised to reflect that.
the project. If the proposal PR defers any documentation changes in this way, | ||
the proposal document should describe what is being proposed in enough detail to | ||
validate that those future PRs properly implement the proposed direction, and | ||
the proposal PR should add "TODO" comments in the places where significant | ||
future changes will be needed (including adding new placeholder documents if | ||
needed), with links back to the proposal document. These comments should be kept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When does this happen in the proposal review process? I see a couple of main directions here (though for each of them we could consider various variations):
-
The PR is written without these TODO comments, reviewed, reaches the state of being signed off. Once everyone is happy and the leads are ready to review, they leave a comment saying the PR is good and it's time to add the TODOs. Advantage: no authoring or review time is wasted on TODO comments that become stale as the design is updated through the review process. Disadvantage: extra (missable) step for the leads to verify the updates have been performed, extra review cycle at the end of the process after the proposal is otherwise ready. (Counterpoint: the extra latency here should have little practical impact because it happens after the leads have indicated they're happy with the proposal.)
-
As the PR is written, the TODOs (or full documentation updates) are added. Checking for documentation updates is part of the normal review cycle. Reviewers shouldn't indicate they're happy until the PR updates the design documentation, either fully or with TODOs. Advantage: distributed work and responsibility makes it more likely that we will remember to do this. Disadvantage: probably somewhat decreased velocity for proposals.
I think it'd be good if we pick one model or the other, so that people have a clear idea of what their responsibilities are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm somewhat inclined toward #2. In addition to the advantages you list, it seems like it might help surface issues during the review, by making it easier to think concretely about how the proposal will affect the rest of the language. It also seems simpler to stay consistent with the general presumption that a proposal is in submittable form throughout the review process.
Co-authored-by: Richard Smith <[email protected]>
Require language design proposals to either update the design documents to
reflect the proposed changes, or add "TODO" comments to mark where those changes
will be needed, with links back to the proposal. This is intended to ensure that
the design documentation accurately informs readers about the current language
design, without excessively burdening the proposal process.