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

Side by Side Diff: docs/v8_committers_responsibility.md

Issue 1347153006: [Docs] Add wiki content to Markdown docs (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 5 years, 3 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 unified diff | Download patch
OLDNEW
(Empty)
1 ## Basic commit guidelines
2
3 When you're committing to the V8 repositories, ensure that you follow those guid elines:
4
5 1. Find the right reviewer for your changes and for patches you're asked to re view.
6 1. Be available on IM and/or email before and after you land the change.
7 1. Watch the [waterfall](http://build.chromium.org/p/client.v8/console) until all bots turn green after your change.
8 1. When landing a TBR change (To Be Reviewed), make sure to notify the people whose code you're changing. Usually just send the review e-mail.
9
10 In short, do the right thing for the project, not the easiest thing to get code committed, and above all: use your best judgement.
11
12 **Don't be afraid to ask questions. There is always someone who will immediately read messages sent to the v8-committers mailing list who can help you.**
13
14 ## Changes with multiple reviewers
15
16 There are occasionally changes with a lot of reviewers on them, since sometimes several people might need to be in the loop for a change because of multiple are as of responsibility and expertise.
17
18 The problem is that without some guidelines, there's no clear responsibility giv en in these reviews.
19
20 If you're the sole reviewer on a change, you know you have to do a good job. Whe n there are three other people, you sometimes assume that somebody else must hav e looked carefully at some part of the review. Sometimes all the reviewers thin k this and the change isn't reviewed properly.
21
22 In other cases, some reviewers say "LGTM" for a patch, while others are still ex pecting changes. The author can get confused as to the status of the review, and some patches have been checked in where at least one reviewer expected further changes before committing.
23
24 At the same time, we want to encourage many people to participate in the review process and keep tabs on what's going on.
25
26 So, here are some guidelines to help clarify the process:
27 1. When a patch author requests more than one reviewer, they should make clear in the review request email what they expect the responsibility of each reviewe r to be. For example, you could write this in the email:
28 ```
29
30 a. larry: bitmap changes
31 b. sergey: process hacks
32 c. everybody else: FYI
33
34 ```
35 1. In this case, you might be on the review list because you've asked to be in the loop for multiprocess changes, but you wouldn't be the primary reviewer and the author and other reviewers wouldn't be expecting you to review all the diff s in detail.
36 1. If you get a review that includes many other people, and the author didn't do (1), please ask them what part you're responsible for if you don't want to re view the whole thing in detail.
37 1. The author should wait for approval from everybody on the reviewer list bef ore checking in.
38 1. People who are on a review without clear review responsibility (i.e. drive- by reviews) should be super responsive and not hold up the review. The patch aut hor should feel free to ping them mercilessly if they are.
39 1. If you're an "FYI" person on a review and you didn't actually review in det ail (or at all), but don't have a problem with the patch, note this. You could s ay something like "rubber stamp" or "ACK" instead of "LGTM." This way the real r eviewers know not to trust that you did their work for them, but the author of t he patch knows they don't have to wait for further feedback from you. Hopefully we can still keep everybody in the loop but have clear ownership and detailed r eviews. It might even speed up some changes since you can quickly "ACK" changes you don't care about, and the author knows they don't have to wait for feedback from you.
40
41 (Adapted from: http://dev.chromium.org/developers/committers-responsibility )
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698