[Gluster-devel] Reviewing patches early

Jeff Darcy jdarcy at redhat.com
Wed Jun 25 17:01:58 UTC 2014


Justin asked me, as the group's official Grumpy Old Man, to send a note
reminding people about the importance of reviewing patches early.  Here
it is.  As I see it, we've historically had two problems with reviews.

(1) Patches that don't get reviewed at all.

(2) Patches that have to be re-worked continually due to late reviews.

We've made a lot of progress on (1), especially with the addition of
more maintainers, so this is about (2).  As a patch gets older, it
becomes increasingly likely that it will be rebased and regression tests
will have to be re-run because of merge conflicts.  This isn't a problem
for features to which Red Hat has graciously assigned more than one
developer, as they review each others' work and the patch gets merged
quickly (sometimes before other interested parties have even had a
chance to see it in the queue but that's a different problem).  However,
it creates a problem for *every other patch*, which might now have to
rebased etc. - even those that are older and more important to users and
up against tighter deadlines.  This "priority inversion" can often be
avoided if people who intend to review a patch would do so sooner, so
that all of the review re-work can be done before new merge conflicts
are created.  Given the differences in time zones throughout our group,
each round of such unnecessary work can cost an entire day, leading to
even more potential for further merge conflicts.  It's a vicious cycle
that we need to break.  Please, get all of those complaints about tabs
and spaces and variable names in *early*, and help us keep the
improvements flowing to our users.


More information about the Gluster-devel mailing list