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 |