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

[Marc Mutz]

On 2017-10-13 16:03, Jedrzej Nowacki wrote:
> If you do like that then you are doing it wrong. Review process is 
> _not_ based
> on a name / company / sun activity. It is  based on the change content. 
> Even
> best people do mistakes.

I'm entirely with Jedrzej here. Do not +2 if you don't understand the 
change - all of it, regardless of who it's from. If you only did a 
cursory review, give a +1, at most, and mention that it's incomplete. If 
you do not understand the header change you used as an example, ask for 
it to be clarified (in the commit message, preferably), since chances 
are that other reviewers or later git history users might have the same 
problems understanding code.

I would even qualify what Thiago wrote in response: I don't want 
Thiago's +2 on a QStringView patch if he didn't understand the code. 
Sure, I don't want unreasonable bikeshedding over subjective matters in 
"my" code, since the author of the patch should have the last say in 
these, but I don't want someone to usher my code through because I 
happen to be the principal author of the code.

Another, slightly related aspect, and one of the reasons why I said 
Viktor got it the wrong way around is that I prefer cross-company 
reviews over intra-company ones, and so should anyone. If you ask a 
member of your own organisation to review a patch, there are social 
effects at work that will make the review less valueable than one done 
by an outsider: unreasonable trust in your colleagues, trying to be 
helpful by unblocking a patch, ...

Consequently, I accept a +2 from a KDABian only if I'm 
above-the-ordinary-ly sure about the correctness of my code. Otherwise, 
I wait for Thiago or Olivier or any at tqc to give a second opinion.

Yes, it's a pain to wait for code review, but office-neighbour reviews 
just tend to produce bad code/apis, and often you can do better by 
writing a page of commit message _more_ than you usually would to 
anticipate and answer questions regarding the code ahead of time.

Thanks,
Marc