[Development] Speeding up the review process (was: PostgreSQL cross compile for Pi)

Thiago Macieira thiago.macieira at intel.com
Fri Oct 13 16:56:28 CEST 2017


On Friday, 13 October 2017 04:04:46 PDT Viktor Engelmann wrote:
>  5. Set a deadline for criticism on the general approach to a change.
>     Too often I have had the situation that I uploaded a patch, then we
>     debated the qdoc entries, variable names, method names, etc FOR
>     MONTHS - and when I thought we were finally wrapping up and I could
>     soon submit it, someone else chimes in and says "this should be done
>     completely differently". Even if the person is right - they should
>     have said that months earlier - before I wasted all that valuable
>     time on variable names, compliance with qdoc guidelines, etc.
>     In earlier discussions I have been told that such a deadline would
>     have to be long, because someone who might have an opinion might be
>     on vacation. IMHO, this doesn't count, because a) you can still make
>     an exception to the rule in such a situation and b) by that logic
>     you should never approve anything, because we also might hire a new
>     employee in 10 years who might have an opinion.

I urge all reviewers to adopt the three-phase review process outlined in Sarah 
Sharp's blog:

	http://sarah.thesharps.us/2014/09/01/the-gentle-art-of-patch-review/

The only problem with this process is that different reviewers will reach 
different stages at different times. So some of them may already have finished 
the review of the concept and may be already providing criticism on the 
implementation, while others are still not convinced of the concept.

Therefore, I add this extra advice on top of the blog:

When you're a reviewer in a change with multiple other reviewers, try to keep 
pace with the other reviewers and not race ahead. If it's not evident where 
the other reviewers are, communication is good. Post something like

"Thank you for your change, I like the idea and I think you implemented it 
properly. I have some comments on your coding style, but we'll discuss that 
when other reviewers have had a chance to review your implementation too."

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center




More information about the Development mailing list