[vtk-developers] Accidental gerrit merge

[Marcus D. Hanwell]

Gerrit says you did, and I couldn't reproduce a bug where clicking
without assigning the score does that automatically. I honestly tried
to make it submit and merge without scoring the topic on a topic we
had that was ready to merge - it kept refusing until I assigned the
score required in Verified and Code Review.

I have no idea what happened in that topic, but I was unable to
reproduce an accidental merge without assigning a +2 review to the
topic.

On Tue, Sep 11, 2012 at 7:50 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
> But I don't think I gave the topic a +2. Am I missing something?
>
>
> On Tue, Sep 11, 2012 at 7:05 PM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
>>
>> That is what I was saying - the change reviews do not matter from a
>> merge point of view. A -1 would never block anyway (-2 required to
>> block). Currently change scores are not able to block topic merges, so
>> even a -2 would not have blocked because it is all on the topic review
>> right now.
>>
>> By giving a +2 to the topic it satisfied all requirements (the topic
>> score is the only score assessed by Gerrit when deciding if a change
>> can be merged or not, robot already gave it +1 Verified).
>>
>> http://review.source.kitware.com/#/t/1250/
>>
>> In future I think any blocker in a change should block a topic from
>> being merged, but we have not had time to implement that feature.
>>
>> Marcus
>>
>> On Tue, Sep 11, 2012 at 6:55 PM, Bill Lorensen <bill.lorensen at gmail.com>
>> wrote:
>> > So here I had a -1: http://review.source.kitware.com/#/c/7465/
>> > How did I merge it?
>> >
>> >
>> >
>> > On Tue, Sep 11, 2012 at 6:52 PM, Bill Lorensen <bill.lorensen at gmail.com>
>> > wrote:
>> >>
>> >> I thought I had a -1 on a change or review. And somehow I merged it.
>> >> Probably a screwup on my part.
>> >>
>> >>
>> >> On Tue, Sep 11, 2012 at 6:45 PM, Marcus D. Hanwell
>> >> <marcus.hanwell at kitware.com> wrote:
>> >>>
>> >>> Sorry if I made them sound too complicated, they are quite simple.
>> >>> Only the topic level review affects whether a topic can be merged or
>> >>> not. In order to qualify for merging a topic must have +1 Verified
>> >>> (set by a robot for VTK) and +2 Code Review (set by a reviewer).
>> >>> Without them it cannot be merged.
>> >>>
>> >>> It gets a little more complex - if you feel a patch is bad you can
>> >>> block it. A score of -1 Verified (set by robot), or -2 Code Review
>> >>> (set by reviewer) will prevent a merge until that score is changed or
>> >>> removed, i.e. even if I give a topic +2 review, if it has a -2 review
>> >>> I cannot merge it  until that score is changed or removed.
>> >>>
>> >>> I hope that is clearer. We would like to link blockers at the
>> >>> individual change level to the topic, but currently the change reviews
>> >>> are summarized in the topic view but are not enforced as rules.
>> >>>
>> >>> Marcus
>> >>>
>> >>> On Tue, Sep 11, 2012 at 6:18 PM, Bill Lorensen
>> >>> <bill.lorensen at gmail.com>
>> >>> wrote:
>> >>> > The rules for a merge sound complicated to me.
>> >>> >
>> >>> > But, I will be very careful in the future.
>> >>> >
>> >>> >
>> >>> > On Tue, Sep 11, 2012 at 6:10 PM, Marcus D. Hanwell
>> >>> > <marcus.hanwell at kitware.com> wrote:
>> >>> >>
>> >>> >> On Tue, Sep 11, 2012 at 5:39 PM, Sean McBride
>> >>> >> <sean at rogue-research.com>
>> >>> >> wrote:
>> >>> >> > On Tue, 11 Sep 2012 17:19:12 -0400, Bill Lorensen said:
>> >>> >> >
>> >>> >> >>I hit the wrong button by accident. I did not give it a +2. I'm
>> >>> >> >> pretty
>> >>> >> >> sure
>> >>> >> >>I gave it (or a change) a -1.
>> >>> >> >
>> >>> >> > IIRC, the topic had several changes, and some of the changes I
>> >>> >> > think
>> >>> >> > were given +2.
>> >>> >> >
>> >>> >> > Is it possible that the whole topic can get merged if any one of,
>> >>> >> > but
>> >>> >> > not all, of the sub-changes are at +2?
>> >>> >> >
>> >>> >> For Gerrit topic review it does not matter what any of the
>> >>> >> individual
>> >>> >> changes get as a review. The entire topic must have a Verified +1,
>> >>> >> and
>> >>> >> a Code Review +2 score in order to be merged. Reviewers can block a
>> >>> >> merge by giving a -2 at the topic level, that topic had a single +2
>> >>> >> given by one user and no other scores.
>> >>> >>
>> >>> >> Any -1 score does not block a commit, any +1 does not enable the
>> >>> >> submit and merge or submit change button. If you review a commit
>> >>> >> and
>> >>> >> it does not have a +1 Verified and +2 Code Review you will see an
>> >>> >> error page informing you the change needs Verified and Code Review.
>> >>> >> I
>> >>> >> tested this myself on a change earlier today, it blocks merging
>> >>> >> until
>> >>> >> both those scores are in.
>> >>> >>
>> >>> >> I can't see any way that an accidental merge can be done in less
>> >>> >> than
>> >>> >> two steps (+2 Code Review score, and then hitting Publish and
>> >>> >> Submit).
>> >>> >> It is possible there is a bug, but I was not able to trigger it
>> >>> >> (and I
>> >>> >> tried a few times).
>> >>> >>
>> >>> >> Marcus
>> >>> >
>> >>> >
>> >>> >
>> >>> >
>> >>> > --
>> >>> > Unpaid intern in BillsBasement at noware dot com
>> >>> >
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Unpaid intern in BillsBasement at noware dot com
>> >>
>> >
>> >
>> >
>> > --
>> > Unpaid intern in BillsBasement at noware dot com
>> >
>
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
>