diff options
| author | Abe Levkoy <alevkoy@chromium.org> | 2020-03-09 16:37:42 -0600 |
|---|---|---|
| committer | Commit Bot <commit-bot@chromium.org> | 2020-03-10 19:37:01 +0000 |
| commit | a4648d636e5d309b3d1882fe688c2eb0dd1d14dd (patch) | |
| tree | d8f3934ecfd0519104ed9b9b5a8d244a5d96c3b0 | |
| parent | 0d44674274c1b6d9bd7c3de90312982883a6e3c1 (diff) | |
| download | chrome-ec-a4648d636e5d309b3d1882fe688c2eb0dd1d14dd.tar.gz | |
docs: Update code review guidelines with feedback
Remove optional ways to indicate WIP to avoid confusion. Add a
1-business-day response SLO for reviewers.
BUG=none
TEST=Viewed rendered Markdown
BRANCH=none
Change-Id: I74411cebf6d51886f845122886fa56c8732336e6
Signed-off-by: Abe Levkoy <alevkoy@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2095749
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Jett Rink <jettrink@chromium.org>
| -rw-r--r-- | docs/code_reviews.md | 13 |
1 files changed, 9 insertions, 4 deletions
diff --git a/docs/code_reviews.md b/docs/code_reviews.md index c3fe407aca..2abe2cad27 100644 --- a/docs/code_reviews.md +++ b/docs/code_reviews.md @@ -29,6 +29,10 @@ should at least ensure that EC style and paradigms are being followed. Once EC styles and paradigms are being followed, then the reviewer can give a +1 and add the appropriate domain expert for that section of code. +Reviewers should try to give an initial response within 1 business day of +receiving a review request. Thereafter, they should try to respond to new +comments by the author within 1 business day. + ## How can I join the rotation? Add your name to the [list of reviewers][1]. @@ -62,10 +66,6 @@ Add your name to the [list of reviewers][1]. review. * Use Gerrit comments as needed to clarify that a CL is now ready for review. * Recommended: Leave “WIP” in the CL title until it is ready to review. - * Optional: Mark the CL as “Work in Progress” in Gerrit (or use `repo upload - --wip`) until it is ready to review. - * Optional: Mark the CL as Verified+1 when it is ready for review. - * Optional: Mark the CL as Verified-1 _until_ it is ready for review. * Make sure that `make buildall` succeeds after each individual change; this facilitates bisecting. * Assign reviewers with a specific scope for each reviewer. @@ -101,8 +101,13 @@ Add your name to the [list of reviewers][1]. * Try to make review comments maximally actionable for authors, who may be in different timezones or may be managing a relation chain with multiple reviewers. + * Try to respond to review requests or follow-up comments within 1 business + day. + * This is an SLO for single responses, not the entire lifecycle of the CL. * Prefer to provide feedback on an entire CL in one shot, so that the author could plausibly respond to all of it and need no further review. + * However, consider sending a reply after a partial review, if it would help + keep the response time within 1 business day. * Resolve outstanding comments before submitting. * Don’t mark a comment as resolved until the question has been answered, request completed, concern assuaged, etc. |