Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(106)

Unified Diff: docs/code_reviews.md

Issue 2932013002: Add examples for some scenarios in the code review docs. (Closed)
Patch Set: format Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: docs/code_reviews.md
diff --git a/docs/code_reviews.md b/docs/code_reviews.md
index 6efe906166b90c994d45006416ac3c822e1ae486..ae6bfd0f19e4f0ddfa192b3ec0eba0ea9d487666 100644
--- a/docs/code_reviews.md
+++ b/docs/code_reviews.md
@@ -43,8 +43,13 @@ get a positive review from an owner of each directory your change touches.
Owners files are recursive, so each file also applies to its subdirectories.
It's generally best to pick more specific owners. People listed in higher-level
-directories may have less experience with the code in question. More detail on
-the owners file format is provided in the "More information" section below.
+directories may have less experience with the code in question. For example,
+the reviewers in the `//chrome/browser/component_name/OWNERS` file will likely
+be more familiar with code in `//chrome/browser/component_name/sub_component`
+than reviewers in the higher-level `//chrome/OWNERS` file.
+
+More detail on the owners file format is provided in the "More information"
+section below.
*Tip:* The `git cl owners` command can help find owners.
@@ -109,27 +114,40 @@ Otherwise the reviewer won't know to review the patch.
like normal.
* Add a line "TBR=<reviewer's email>" to the bottom of the change list
- description.
+ description. e.g. `TBR=reviewer1@chromium.org,reviewer2@chromium.org`
+
+ * Type a message so that the owners in the TBR list can understand who is
+ responsible for reviewing what, as part of their post-commit review
+ responsibility. e.g.
+ ```
brettw 2017/06/09 15:31:17 I'm not sure gitiles will do what you want here, b
Lei Zhang 2017/06/09 21:52:50 Seems to work: https://chromium.googlesource.com/c
+ TBRing reviewers:
+ reviewer1: Please review changes to foo/
+ reviewer2: Please review changes to bar/
+ ```
* Push the "send mail" button.
### TBR-ing certain types of mechanical changes
Sometimes you might do something that affects many callers in different
-directories. For example, adding a parameter to a common function in //base.
-If the updates to the callers is mechanical, you can:
+directories. For example, adding a parameter to a common function in
+`//base`, with callers in `//chrome/browser/foo`, `//net/bar`, and many other
+directories. If the updates to the callers is mechanical, you can:
- * Get a normal owner of the lower-level directory you're changing (in this
- example, `//base`) to do a proper review of those changes.
+ * Get a normal owner of the lower-level code you're changing (in this
+ example, the function in `//base`) to do a proper review of those changes.
- * Get _somebody_ to review the downstream changes. This is often the same
- person from the previous step but could be somebody else.
+ * Get _somebody_ to review the downstream changes made to the callers as a
+ result of the `//base` change. This is often the same person from the
+ previous step but could be somebody else.
- * Add the owners of the affected downstream directories as TBR.
+ * Add the owners of the affected downstream directories as TBR. (In this
+ example, reviewers from `//chrome/browser/foo/OWNERS`, `//net/bar/OWNERS`,
+ etc.)
This process ensures that all code is reviewed prior to checkin and that the
concept of the change is reviewed by a qualified person, but you don't have to
-track down many individual owners for trivial changes to their directories.
+wait for many individual owners to review trivial changes to their directories.
### TBR-ing documentation updates
@@ -155,7 +173,7 @@ comments inside functions.
### OWNERS file details
Refer to the [source code](https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/owners.py)
-for all details on the file format.
+for all details on the file format.
This example indicates that two people are owners, in addition to any owners
from the parent directory. `git cl owners` will list the comment after an
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698