Re: How does the review process work..?

110 views
Skip to first unread message

Jean-Baptiste Queru

unread,
Jun 9, 2013, 12:35:36 PM6/9/13
to android...@googlegroups.com
1. You don't actually have to explicitly specify the reviewers, though it's advisable. There's also a bot (Deckard) that does this in the vast majority of projects, and (time allowing) humans also look at all pending changes one by one to catch those that fall off the process. You can find the lists of owners in Gerrit, under (IIRC) People -> List Groups. The projects that don't have an explicit group don't have restrictions and are owned by all Android Maintainers.

2. Code review = high-level decision, i.e. the change does something that is desirable, the edge cases are properly handled, the algorithmic complexity is OK, etc. Verification = the code style is OK, the code merges, builds, and (ideally) passes all tests. Typically, the former can only be done by someone familiar with the code, and the latter can be done with a bot.

3. +2 = approval, -2 = veto (the strongest), whereas +1 and -1 are just "good idea" and "bad idea" with no actual power. The numbers don't add up, so no amount of +1s can turn into a +2 or counter a -2.

4. +2 is the approval. In order for a maintainer to be able to submit the change, it needs at least one CR:+2 and one V:+1, with no CR:-2 and no V:-1.

JBQ


On Sun, Jun 9, 2013 at 8:28 AM, <and...@howardb.com> wrote:
Hi there. I'm new to this contributing process, but I'm struggling to find information about how "reviewing" exactly works. I've tried reading old posts, but it's clear the process keeps evolving.

I'd appreciate some help or answers for some of the following - I doubt I'm the only one with these questions. I'd be willing to help raise a patch for the docs with anything I learn that can help newbies (though I notice the contrib docs in AOSP doesn't actually match the contrib docs on the web anyway?)

1. Can you make it clearer on the http://s.android.com/source/submit-patches.html page that you have to explicitly add the reviewers yourself..? It took me a while to work that out and - looking back through posts - I'm not the only one. Some git commands such as log or blame were helpful here. On this subject, the Chromium project uses OWNERS files in the root of folders or projects. These OWNERS files hold the names of the people who can review code changes for those files/packages and are automatically added to new changes. Has anyone thought about doing something similar for *some* of the folders or packages in the AOSP structure..?

2. What is the difference between "code-review +1" and "verified"? Is one dependant on the other? Who performs the "verified" step..? I've seen mentions of "Deckard" but I don't understand if I am meant to add it, when and what it does? Is it the final step for approval of the whole change? Do all types of changes require verification?

3. What does "+2" mean? I've seen mentions of it, but I don't understand if it means two people have +1'd your change or if a single uber-reviewer has approved your change and you don't need any other approvals?

4. Do you need to get a +2 before your change is approved? Or just a single +1? Or two separate +1s..? I'm confused..

Thanks for any answers you can give!

--
 
---
You received this message because you are subscribed to the Google Groups "Android Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email to android-contr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 



--
Jean-Baptiste M. "JBQ" Queru
Technical Lead, Android Open Source Project, Google.

Questions sent directly to me that have no reason for being private will likely get ignored or forwarded to a public forum with no further warning.
Reply all
Reply to author
Forward
0 new messages