13.10.2017, 14:05, "Viktor Engelmann" <viktor.engelm...@qt.io>: > 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 > > * Allow approving your own commits under certain circumstances. examples: > > * when you only changed minor things like a variable name (except in public > API), a string, a comment or qdoc entry > * 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 )
FWIW, in WebKit project it's a usual workflow to set "r+" and at the same time request minor changes to the patch, fixed version of patch is submitted without new review. > * 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. > * or more general: when you only made an autotest harder to pass - like > adding a Q_VERIFY, Q_COMPARE, etc. > * when the change is something auto-generated - like you just updated > plugins.qmltypes using qmlplugindump > * when you only changed something in accordance to the guidelines - like > Q_DECL_OVERRIDE -> override > * when you have a certain count of +1's from people who have approver rights Note that at least some of this rules can be implemented in gerrit itself, so you don't need self-approve https://codereview.qt-project.org/Documentation/prolog-cookbook.html https://codereview.qt-project.org/Documentation/prolog-change-facts.html > * 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! > * 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. > * I don't think we need to be as paranoid towards contributions from our own > employees as we need to be towards external contributions. Hey, it's a discrimination! > * 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.engelm...@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 > , > > _______________________________________________ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development -- Regards, Konstantin _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
