[Development] Review process: when should I merge to staging?

[Alan Alpert]

On Mon, Aug 12, 2013 at 1:42 PM, Josh Faust <jfaust at suitabletech.com> wrote:
>
>>
>> Thiago covered it expertly, but I'd also suggest giving it some more
>> time if specific other reviewers have expressed interest in it (such
>> as by repeated comments). In a case like this one, where it has been
>> entirely a dialogue with the person who +2'd it, no waiting period
>> seems necessary.
>
>
> To be clear: in the reviewer table, a checkmark is +2/approved? I've  seen
> +1s in the gerrit UI but never a +2.

Correct.

>
>>
>>
>> I noticed this morning that I forgot to +1 sanity review when I
>> approved it. If the sanity bot comments have been covered in the
>> discussion already, like in that patch, just go ahead and +1 sanity
>> review it yourself if it has +2 code review. The -1 ratings from the
>> sanity bot do not require a separate approver to override, it's just
>> raising a red flag that should be addressed in the review before it's
>> given +2.
>
>
> I've never seen the sanity review options -- do they only show up if the
> Sanity Bot -1'd it? Or do I just merge without Sanity Bot approval?

When you click review, there is a "Sanity Review" set of option
buttons underneath the "Code Review" set. However it is folded up by
default, you need to unfold it to see the options. Being slightly
hidden like this is why it's relatively common for reviewers to forget
to +1 the sanity when they meant to along with a +2.

You need a +1 sanity review to merge, but it doesn't need to come from
the Bot. The sanity review column is a bit confusing, but a +1 (tick
mark) is needed in it for staging and -1s are then ignored (same as a
-1 code review, except the bot has no feelings to hurt ;) ).

--
Alan Alpert