[vtk-developers] Accidental gerrit merge

Marcus D. Hanwell marcus.hanwell at kitware.com
Tue Sep 11 19:05:49 EDT 2012


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
>



More information about the vtk-developers mailing list