On 7/11/13 9:23 AM, Justin Lebar wrote:
One thing I've been thinking about is /why/ people are slow at reviews.
Here are some possible reasons I've encountered myself:
1) Feeling overwhelmed because you have too many reviews pending and
therefore just staying away from your list of reviews. Note that this
can be self-perpetuating.
2) Feeling like you have to do your reviews in some sort of FIFO order,
so that a review for a large patch (which takes a long time because
that's just how large patches work) will block possibly-quick reviews
for smaller patches.
3) Feeling like you want to do your reviews in some sort of
most-expeditious order, so that a constant stream of small patches
delays review of a big patch for weeks or months.
4) Feeling that you need multiple hours of uninterrupted time, probably
over several days for review of a complicated patch and not being able
to find it due to the scattering of meetings, people asking you
questions, smaller reviews, etc that you're also doing. Note that
anyone who has no time to code because of reviews probably also has no
time to do big complicated reviews, for the same reasons!
5) Simply having too high a review load. Anecdotally, it typically
takes me until sometime Tuesday afternoon at best to catch up on all the
review requests that come in between end-of-work Friday and Monday morning.
6) Just disliking doing reviews (but more on this below).
7) Wanting to completely focus on some other complicated task for a few
days ... or weeks.
It would be interesting to see what other reasons there are and what the
relative frequencies are.
Someone who usually has a long review queue has told me that he "hates"
reviewing code.
Reviewing code is just not all that fun. You have to tell people they
did something wrong, often with some fairly arbitrary (in the grand
scheme of things; we do it so we have consistent coding style) nitpicks,
there is not that much of a sense of accomplishment at the end. It
feels very adversarial at times. Sometimes you learn new things in the
process, but more often you try hard to politely tell people that
they've screwed something up... possibly after you already told them
that about that same thing in the previous iteration of the patch. At
least for me, it can get downright depressing at times.
But debugging leaks is not that fun either. Or trying to read through
flat profiles and speed things up. Or hunting down a dangling pointer
bug. Or trying to convince people on a standards mailing list to not do
something insane, for that matter.
Fundamentally, we feel that reviews are useful and that means they have
to get done; the doing is not all that fun, but it's a sort of giving
back to the community, in my view.
I realized that we don't really have a place at Mozilla for
experienced hackers who don't want to do reviews. Should we? Could we do this
without violating people's sense of fairness?
How do we handle this with the other unpalatable tasks above?
Informally, what I see is that they get shunted to people who are either
not as unhappy doing them or have a stronger sense of "I don't like it,
but it needs doing, and no one else is stepping up". And it sure as
heck violates people's sense of fairness. Of course we do that with
reviews too (a well-known technique for doing fewer reviews is just
being slow about it so people stop asking, whereas someone who does
reviews quickly is "rewarded" by people piling on reviews), and it
likewise violates people's sense of fairness.
Of course the other thing I see happen with the unpalatable tasks I list
above is that they just don't get done in many cases because no one
steps up. That's a bit rarer with reviews, because we don't consider
those as optional. But it has happened: I've seen patches die because no
one stepped up to review.
Here's another way to look at this: do these hackers want _others_ to
review their patches? If so, how do they expect that to happen?
Obviously everyone would prefer to work on the things they _want_ to
work on, not necessarily the things that _need_ to be worked on. To the
extent that someone focuses more on the former than on the latter,
they're shafting others whose sense of duty won't let them do that. The
result is that you get people who are unhappy because they never get to
work on the things they want to or who are unhappy because they're doing
all their "I would like to get this done" work after hours. Or both, at
times: only working on things that need to happen _and_ having to do it
after hours...
And as a further note, the more reviews pile on just a small subset of
people, the more problems 1-5 listed above manifest. As Taras said, the
right solution to review load is to grow more reviewers, but that's hard
to do if people are basically shirking the responsibility.
As another data point, someone told me recently that he's trying not to become
a peer of the modules he's working on, because he doesn't want to review code.
But he wants his own code reviewed, right? ;)
Maybe we're not incentivizing people enough to be reviewers.
That's very much possible, yes. It's not clear to me how to best
incentivize this without creating incentives for rubber-stamping code.
-Boris
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
