[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