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

[Sergio Ahumada]

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
https://codereview.qt-project.org/#admin,project,All-Projects,access

> 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
https://qt.gitorious.org/qtqa/gerrit/commits/v2.2.1-based


-- 
Sergio Ahumada
san at sansano.inf.utfsm.cl