[Gluster-devel] Reviewing patches early

[Pranith Kumar Karampuri]

On 06/25/2014 10:31 PM, Jeff Darcy wrote:
> 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
While I agree with everything you said. Complaining about tabs/spaces 
should be done by a script. Something like 
http://review.gluster.com/#/c/5404
Some one who knows perl should help us with rebasing it and getting it in?

Pranith
> improvements flowing to our users.
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel