Add Design Doc policy #115
Conversation
designs/index.md
Outdated
| ## When do I need a design document and review? | ||
|
|
||
| If you're planning to add, change, or remove a user-facing feature, or make a | ||
| significant architectural change to Bazel, you must write a design document and |
designs/index.md
Outdated
| change?](#what-is-a-significant-change) for details. | ||
|
|
||
| Implementation can begin before the proposal is accepted, for example as a | ||
| proof-of-concept or an experimentation. However, the change may not be effective |
There was a problem hiding this comment.
"However, you cannot submit the change before the review is complete."?
There was a problem hiding this comment.
Not submit makes more sense thatn "may not be effective"
designs/index.md
Outdated
| ### Create a Pull Request | ||
|
|
||
| The author creates a PR to add the document to [the design index](https://github.com/bazelbuild/proposals). If | ||
| the proposal is a markdown file, the file is added in the same PR. Otherwise, |
There was a problem hiding this comment.
I don't think this works in practice. You want to add proposals to the index while the content of the proposal is still being reviewed and debated. When the proposal moves from stage to stage, we update the index.
Consider a proposal as a Google doc. We add the pointer to the draft to the index. Then we discuss the proposal. When it gets approved or rejected, we update the index.
designs/index.md
Outdated
| ### Update the status | ||
|
|
||
| When the author believes the iteration round is complete, they send a new PR to | ||
| update the status. The PR must be sent to the lead reviewer and cc the other |
designs/index.md
Outdated
| update the status. The PR must be sent to the lead reviewer and cc the other | ||
| reviewers. | ||
|
|
||
| The lead reviewer approves the PR to officially accept or reject the proposal. |
There was a problem hiding this comment.
"The lead reviewer approves the PR to officially accept the proposal."
designs/index.md
Outdated
|
|
||
| ## How to choose a lead reviewer? | ||
|
|
||
| The lead reviewer is a domain expert. Lead reviewers must be: |
designs/index.md
Outdated
|
|
||
| * Knowledgeable of the relevant subsystems | ||
| * Objective (i.e., capable of providing constructive feedback) | ||
| * Have sufficient time available to lead the review process |
There was a problem hiding this comment.
Lead reviews must be: available for the entire review period to lead the process
| * Changes to flags and command-line interface. | ||
|
|
||
| When a proposal adds, removes, or modifies any function or object available in | ||
| BUILD, WORKSPACE, or bzl files, Skylark team has to be in the reviewers list. |
There was a problem hiding this comment.
Should we make a list of team members and their GitHub aliases?
There was a problem hiding this comment.
Yes, we need this information on the website
designs/index.md
Outdated
| * Bazel is a very complex system; seemingly innocuous local changes can have | ||
| significant global consequences. | ||
| * The team gets many feature requests from users; such requests need to be | ||
| evaluated not only for technical feasibility but importance w.r.t. other |
designs/index.md
Outdated
| such contributors have widely varying levels of Bazel expertise. | ||
| * The Bazel team itself has varying levels of expertise; no single team member | ||
| has a complete understanding of every corner of Bazel. | ||
| * Changes to Bazel need to pay attention to backward compatibility and avoid |
There was a problem hiding this comment.
s/need to pay attention to/must account for/
|
Thanks for the review! |
designs/index.md
Outdated
| * Bazel features are frequently implemented by people outside the core team; | ||
| such contributors have widely varying levels of Bazel expertise. | ||
| * The Bazel team itself has varying levels of expertise; no single team member | ||
| has a complete understanding of every corner of Bazel. | ||
| * Changes to Bazel need to pay attention to backward compatibility and avoid | ||
| breaking changes. | ||
| * Changes to Bazel must account to backward compatibility and avoid breaking |
designs/index.md
Outdated
| @@ -144,11 +144,12 @@ approved by anyone. | |||
|
|
|||
| ## How to choose a lead reviewer? | |||
|
|
|||
| The lead reviewer is a domain expert. Lead reviewers must be: | |||
| The lead reviewer is a domain expert. Lead reviewers must be: | |||
|
|
|||
| * Knowledgeable of the relevant subsystems | |||
| * Objective (i.e., capable of providing constructive feedback) | |||
| * Have sufficient time available to lead the review process | |||
There was a problem hiding this comment.
oops, thanks!
Fixed
designs/index.md
Outdated
| the PR only adds a link. | ||
|
|
||
| When possible, the author [chooses a lead reviewer](#how-to-choose-a-lead-reviewer). | ||
| Other reviewers are cc'ed. If the author didn't choose a lead reviewer, the |
There was a problem hiding this comment.
We should allow for teams to be mentioned as reviewers. Then it is come one, come all. The team lead should be able to assign reviewer lead-reviewer rather than the sheriff. They will have more context.
| Markdown files have some other benefits, including: | ||
|
|
||
| * Clean URLs for linking. | ||
| * Explicit record of revisions. |
There was a problem hiding this comment.
Should there be revisions after the review stage?
Of course there should be. But there are really two phases to any design.
At the beginning there should be discussion and collaboration. Google docs works well for that. After approval, we could freeze what was agreed on in markdown. Any comments in the doc should be brought in as concerns or alternatives and made part of the text.
After implementation, one may want to update the design to reflect implementation drift from design. So that might be worth capturing as changes.
| ## Using Google Docs | ||
|
|
||
| To make a [Google Doc](https://docs.google.com) world-readable, create it with a | ||
| personal account. To share it, click on "Share", "Advanced", then "Change…", and |
There was a problem hiding this comment.
I have trouble with 'personal'. It leaks personal identity into a business setting. Most of the people working on Bazel do it as part of their jobs, not as a personal quest.
Is the intent to mean "if your company uses gSuite, then create a doc with another identity, so that the doc can be shared outside that gSuite domain"?
I suspect that is the motivation, since most admins look down their account to prevent sharing outside it.
There was a problem hiding this comment.
I suspect that is the motivation, since most admins look down their account to prevent sharing outside it.
Correct, like how we use @bazel.build accounts.
| * Changes to flags and command-line interface. | ||
|
|
||
| When a proposal adds, removes, or modifies any function or object available in | ||
| BUILD, WORKSPACE, or bzl files, Skylark team has to be in the reviewers list. |
There was a problem hiding this comment.
Should you clarify that the intent is modifies ... available in all bzl files, rather than
adds a function to any specific .bzl file(s).
No description provided.