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

[Alan Alpert]

On Tue, Aug 13, 2013 at 2:22 AM, Oswald Buddenhagen
<oswald.buddenhagen at digia.com> wrote:
> On Mon, Aug 12, 2013 at 02:32:34PM -0700, Alan Alpert wrote:
>> 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.
>>
> actually, it's a very useful step. people are trying to override the bot
> all the time because they don't get the intentions behind a particular
> report

Or they get that the intention of a heuristic it to guess and realize
that it's not always right?

If you're so certain that the sanity bot is always correct. Why don't
you just make it +/- 2 in the code review? Then we can fight about it
every time I want to get a change in :P .

> (or because they enjoy outsmarting the system just for the sake
> of it, like you apparently do)

We *define* the system. The fact that I have to work around it in
order to do my job properly shows that the system is defective and
needs to be rectified. Lars mentioned at the contributor summit that
we need to make the review process easier for people, and fixing the
system to have fewer unnecessary hurdles is an obvious way to do that.

A more effective change which I recommend would be to have the bot not
give -1 on the heuristics which are known to give false positives more
frequently. It's enough that it provides a warning comment, allowing
the contributors to address the issue if legitimate. But the -1 can
also be viewed as a hint that there's relevant output, since it's so
prominent in the gerrit interface, so the -1 isn't a problem so long
as it doesn't slow us down.

>. having an approver (i.e., somebody who
> is expected to be familiar with the coding style, commit policy, etc.)
> take the decision to override the bot is very much part of the plan.

The point is that the approver has already decided to approve. We
trust the approvers already to read and act on review comments, any
case where they +2'd something which shouldn't go in was a mistake. So
if they've already considered the bot's comments and +2'd the change
that *is* an explicit override. If we could have a reliable system
where it gives you a second box to click on the same page then that
makes sense, but we've had reports of accidents on both sides of the
"sanity review visible" topic. So why should we be forced into a
situation without a good solution, when the whole step is quite
unnecessary?

--
Alan Alpert