[Development] Proposal: Allow contributors to +1 sanity review.

Alan Alpert 416365416c at gmail.com
Mon Aug 12 23:32:34 CEST 2013


I was not aware of this, but given that contributors can now stage
their own changes I'd like it if they had the option to +/- 1 the
sanity review as well. This isn't affecting the approver approval
requirement, as +1 sanity review isn't enough to merge and only
approvers should be able to give -2 sanity review (not that there's
any point to it). Anyone giving themselves erroneous +1s won't hide
the sanity bot either, as the web interface prioritizes -1's in the
summary and the actual sanity bot message (the important part) doesn't
get deleted.

What this will affect is a better implementation of the "contributors
can stage their own changes". In cases where the approver gives +2
approval but forgets the sanity review override (because it's hidden)
they will have implicitly meant to override. Which almost happened
today. I don't think there's a valid usecase where you approve the
code but deliberately want some other approver's opinion to override
the bot heuristics. So all that happens is that there will be cases
where the bot gives a false positive and contributors need to ping
again for sanity review - a needless step.

It would also be nice to show our contributors that we trust them
more, not less, than a bundle of cheap heuristics ;) .

If sanity permissions are not granted to contributors, then I'd like
to fix the web form so the sanity review isn't hidden, to decrease the
chance of it getting skipped by accident. It has happened to others as
well as I, although it might just be the section of code I'm in has
more false positives (qtdeclarative gets false positives fairly
commonly from A) "Missing license header" because QML files in
autotests do not have headers. B) "Adding large file" because our
tests/examples have a lot of images). Every time I have to un-collapse
that section I feel like someone hates me, but I know the bot/UI was
just programmed like that and doesn't bear real malice :P .

Note: I'm happy to take the first shot at implementing such simple
changes to our gerrit code, if the policy decision is made.

--
Alan Alpert



More information about the Development mailing list