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 _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development