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

Sergio Ahumada san at sansano.inf.utfsm.cl
Mon Aug 12 23:47:15 CEST 2013

On 08/12/2013 11:32 PM, Alan Alpert wrote:
> 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 ;) .

Giving everybody the option to +/- 1 the sanity review is a very simple
change in

> 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.

If you want to do this, you would need to revert
782740a58ce87f4ebe1ec5b9b60dbe5978315cc2 and maybe
4a9b21d6f79664fa057dadf4e9653f76497726e1 from

Sergio Ahumada
san at sansano.inf.utfsm.cl

More information about the Development mailing list