[Development] Speeding up the review process (was: PostgreSQL cross compile for Pi)
viktor.engelmann at qt.io
Fri Oct 13 13:04:46 CEST 2017
On the [Interest] mailing list there was a discussion about the
review-process taking to long and we also had multiple discussions about
that at the world summit. I have complained about this myself, so I
would like to start a new thread and collect your thoughts and ideas on
how to improve the situation.
My suggestions would be
1. Allow approving your own commits under certain circumstances. examples:
1. when you only changed minor things like a variable name (except
in public API), a string, a comment or qdoc entry
2. when you already had a +2 and only changed minor things like a
variable name, a string, a comment or qdoc entry (or more
generally: when you already had a +2 and only did something that
is also on this list :-D )
3. when you only increased a timeout in an autotest. Increasing
timeouts is a safe change - it can only solve false negatives
and false positives, but not create false positives or false
4. or more general: when you only made an autotest harder to pass -
like adding a Q_VERIFY, Q_COMPARE, etc.
5. when the change is something auto-generated - like you just
updated plugins.qmltypes using qmlplugindump
6. when you only changed something in accordance to the guidelines
- like Q_DECL_OVERRIDE -> override
7. when you have a certain count of +1's from people who have
2. Towards that last point: I think many of us are afraid to get blamed
for +2'ing something that causes problems later (introduces a new
bug or so), but as far as I have seen, nobody gets blamed for such
problems, so we should not be THAT afraid of approving something.
Also, don't forget that there is still the CI to get past!
3. Remember that brain-cycles are far more expensive than CPU cycles -
so when in doubt, rather test-run something on the CI than make a
human think about whether the CI "would" accept it. If that causes
CI outages, we need to buy more CI machines. It is just a naive
fallacy to "save" CI expenses by assigning the CI's work to
employees who are much more expensive.
4. I don't think we need to be as paranoid towards contributions from
our own employees as we need to be towards external contributions.
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.
The Qt Company GmbH
Rudower Chaussee 13
Viktor.Engelmann at qt.io
+49 151 26784521
Geschäftsführer: Mika Pälsi,
Juha Varelius, Mika Harjuaho
Sitz der Gesellschaft: Berlin,
Charlottenburg, HRB 144331 B
The Future is Written with Qt
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Development