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

[Viktor Engelmann]

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
        negatives.
     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
        approver rights
 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.


-- 

Viktor Engelmann
Software Engineer

The Qt Company GmbH
Rudower Chaussee 13
D-12489 Berlin
Viktor.Engelmann at qt.io
+49 151 26784521
http://qt.io

Geschäftsführer: Mika Pälsi,
Juha Varelius, Mika Harjuaho
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht
Charlottenburg, HRB 144331 B

The Future is Written with Qt
www.qtworldsummit.com

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