Skip to content

Commit 064574c

Browse files
Gabriel CharetteCommit Bot
Gabriel Charette
authored and
Commit Bot
committed
[Code Reviews Docs] Update TBR policy for mechanical changes
As discussed @ https://groups.google.com/a/chromium.org/d/topic/chromium-dev/829Jj5eBhbk/discussion [email protected] Change-Id: I6a0d4a2d4d3091a48cc6c66994f4842f69aa3a87 Reviewed-on: https://chromium-review.googlesource.com/c/1340262 Commit-Queue: Gabriel Charette <[email protected]> Reviewed-by: danakj <[email protected]> Cr-Commit-Position: refs/heads/master@{#609066}
1 parent 8a9ca86 commit 064574c

File tree

1 file changed

+19
-10
lines changed

1 file changed

+19
-10
lines changed

docs/code_reviews.md

+19-10
Original file line numberDiff line numberDiff line change
@@ -217,20 +217,29 @@ different directories. For example, adding a parameter to a common function in
217217
`//base`, with callers in `//chrome/browser/foo`, `//net/bar`, and many other
218218
directories. If the updates to the callers is mechanical, you can:
219219
220-
* Get a normal owner of the lower-level code you're changing (in this
221-
example, the function in `//base`) to do a proper review of those changes.
220+
1. Get a normal owner of the lower-level code you're changing (in this
221+
example, the function in `//base`) to do a proper review of those changes.
222222
223-
* Get _somebody_ to review the downstream changes made to the callers as a
224-
result of the `//base` change. This is often the same person from the
225-
previous step but could be somebody else.
223+
2. Get _somebody_ to review the downstream changes made to the callers as a
224+
result of the `//base` change. This is often the same person from the
225+
previous step but could be somebody else.
226226
227-
* Add the owners of the affected downstream directories as TBR. (In this
228-
example, reviewers from `//chrome/browser/foo/OWNERS`, `//net/bar/OWNERS`,
229-
etc.)
227+
3. TBR the owner of the lower-level code you're changing (in this example,
228+
`//base`), after they've LGTM'ed the API change, to bypass owners review of
229+
the API consumers incurring trivial side-effects.
230230
231231
This process ensures that all code is reviewed prior to checkin and that the
232-
concept of the change is reviewed by a qualified person, but you don't have to
233-
wait for many individual owners to review trivial changes to their directories.
232+
concept of the change is reviewed by a qualified person, without having to ping
233+
many owners with little say in the trivial side-effects they incur.
234+
235+
**Note:** The above policy is only viable for strictly mechanical changes. For
236+
large-scale scripted changes you should:
237+
238+
1. Have an owner of the core change review the script.
239+
240+
2. Use `git cl split` to shard the large change into many small CLs with a
241+
clear description of what each reviewer is expected to verify
242+
([example](https://chromium-review.googlesource.com/1191225)).
234243
235244
#### Documentation updates
236245

0 commit comments

Comments
 (0)