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

[Alan Alpert]

On Tue, Aug 13, 2013 at 1:44 PM, André Pönitz
<andre.poenitz at mathematik.tu-chemnitz.de> wrote:
> On Tue, Aug 13, 2013 at 10:51:17AM -0700, Alan Alpert wrote:
>> On Tue, Aug 13, 2013 at 10:45 AM, Thiago Macieira
>> <thiago.macieira at intel.com> wrote:
>> > On terça-feira, 13 de agosto de 2013 10:26:42, Alan Alpert wrote:
>> >> > (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.
>> >
>> > I don't see what's wrong with the current system. Let's keep it.
>> >
>> > The bot usually doesn't make a mistake.
>>
>> I run into it fairly often.
>> https://codereview.qt-project.org/#q,reviewer:aalpert%2540blackberry.com+status:merged,n,z
>> shows five times this month that it needed to be overridden in my
>> reviews.
>
> I am sure we are not on the same page here. The first patch on the
> list there shows that you approved new(!) code containing things like
>
>    QQuickWindow* window = ...
>
> and
>
>    for (int i = 99; i < 102; ++i)
>    {
>        ...
>
> whereas the Qt Coding Style (http://qt-project.org/wiki/Qt_Coding_Style)
> clearly states
>
>    For pointers or references, always use a single space between the type
>    and ‘*’ or ‘&’, but no space between the ‘*’ or ‘&’ and the variable name
>
> and
>
>    Use attached braces: The opening brace goes on the same line as the
>    start of the statement.

Maybe you can take Oswald up on his offer and add that to the sanity
bot checks then.

To be clear, this discussion is not about altering the functionality
of the sanity bot. The current discussion is about the Gerrit
interface to the sanity review field. If there were any use for that
field other than the bot, then we wouldn't end up having the two
conversations so closely tied together.

--
Alan Alpert