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

[Tuukka Turunen]

Hi,

I hope people can look into Viktor’s complete proposal and think about what is good and what not. This is what we should do when someone makes a proposal to improve things. I think the idea is very good - being more efficient.

What happened in this thread is taking only some parts of it and starting to give completely irrelevant accusations (in some of the earlier responses). Result of such behavior is rarely anything good. At least the person who tried to improve things feels bad and often the main point gets forgotten.

We had a good discussion at Qt Contributor Summit about code of conduct for the Qt Project. I very much hope we are able to create that quickly. It will hopefully help people to speak without constant fear of others taking their words and starting to beat the person making a suggestion.

Yours,

                 Tuukka
_____________________________
From: Marc Mutz <marc.mutz at kdab.com<mailto:marc.mutz at kdab.com>>
Sent: maanantaina, lokakuuta 16, 2017 12:41 ip.
Subject: Re: [Development] Speeding up the review process (was: PostgreSQL cross compile for Pi)
To: <development at qt-project.org<mailto:development at qt-project.org>>


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



_______________________________________________
Development mailing list
Development at qt-project.org<mailto:Development at qt-project.org>
http://lists.qt-project.org/mailman/listinfo/development


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20171016/c4580941/attachment.html>