Getting faster owners reviews

[Brett Wilson]

Some of the more popular directories in Chrome have, by their nature,
a limited set of engineers as reviewers. These people tend to be
overloaded. This makes your review get backed up and everybody is
unhappy.

The owners system exists because not everybody might have the
experience in an area they're changing to know whether a certain type
of change is appropriate. BUT: all committers should be qualified to
review any change for code clarity and correctness! And one of the
best ways to get more senior is to do lots of code reviews! So if
you're making changes to a popular directory (base, content, etc.),
consider:

#1. Setting somebody appropriate who's not an owner to the code
review. It could be somebody who you discussed your patch with,
somebody knowledgeable in that area but not an OWNER, etc. Hopefully
this is not somebody you know will just rubberstamp whatever you send
since then step #4 won't work.

#2. Getting the code reviewed from that person for code architecture,
correctness, clarity, and style. This reviewer learns what you are
doing and becomes more knowledgeable and experienced for future
reviews.

#3. LATER adding an owner to the code review for the higher-level
owners review. Adding later is critical, since otherwise that person
will see a 6 email thread that keeps popping up in their inbox and
probably ignore it, not knowing when it's ready for final approval.

#4. Getting quick LGTM from the owner who hopefully agrees with your
overall design and knows they don't have to check for memory leaks,
off-by-one errors, and comments missing periods at the end of the
sentence :)

#5. If there are design-level comments,.both you and your original
reviewer can learn something new for next time, leading to a virtuous
cycle of awesomeness!

If you're unsure about something, start with:

#0. Ask owner if a certain approach is appropriate.

Most people will respond much faster to a direct email with a specific
question ("I want to add a Lisp interpreter to base, is that
appropriate?") than a code review ("Convert preferences system to
parenthesized format"). And when somebody's already been given a
heads-up on some change, and they see a code review that's already
been thoroughly checked by your awesome co-worker, your speedy
LGTM is assured.

[Peter Kasting]

On Tue, Mar 20, 2012 at 11:21 PM, Brett Wilson <bre...@chromium.org> wrote:

#1. Setting somebody appropriate who's not an owner to the code
review.

I want to call this out in a slightly different way, using a tip I give in my Noogler talk.

If you are the sole owner (in the broader sense) of a piece of code, consider bringing someone else up to speed on it by slowly diverting code reviews their way (after checking with them first).  Similarly, when deciding what to work on, try to seek out new, unfamiliar areas of the codebase every couple of months, and become more familiar with them, maybe even becoming a co-owner over time.

These aren't natural practices to many of us, since we naturally tend to specialize and become experts on isolated fiefdoms.  But this sort of breadth of knowledge pays off in many ways, from ensuring code has multiple owners in case one is busy or goes on leave, to spreading useful knowledge and best practices more broadly, to reducing the chance that something subtle breaks because no one understood the big picture well enough.

PK

[John Abd-El-Malek]

+1 to everything that's been said here.

One more thing: this is also a great way to add more OWNERS to existing directories, especially where we don't have OWNERS in other timezones. After a bunch of reviews by the person in #1 that show an understanding of a particular directory, then that person proves that they're qualified to be added as an OWNER. Note that this doesn't necessarily mean that they have to be familiar with all the code in the directory. But they have to make good judgement calls of when they're not qualified to review something.

On Tue, Mar 20, 2012 at 11:21 PM, Brett Wilson <bre...@chromium.org> wrote:

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev