[Insight-developers] Failed to merge a gerrit patch

Mon Aug 15 11:53:08 EDT 2011

I agree with Bill.

I often submit patches for others.  Gerrit is great, but if we allow the process to involve too many serialized steps, it ends up slowing down the whole process.

If there is a minor patch (say to fix a compiler warning on windows), and I ask someone with a windows compiler (say Bill L) to test it, then it is really nice for Bill to just merge the patch for me once he has determined it is working.

Most saturday mornings, I spend a good chunk of time reviewing Gerrit tickets.  If they are good, pass tests, and are reviewed, then I take it upon myself to ease the burden of the prime algorithm developers by merging their approved patches for them.  There are several of us doing this, and it is a great benefit to the community to improve efficiency of getting the dashboard green.

Hans
--
Hans J. Johnson, Ph.D.
hans-johnson at uiowa.edu<mailto:hans-johnson at uiowa.edu>
Assistant Professor of Psychiatry
University of Iowa Carver College of Medicine
W278 GH, 200 Hawkins Drive
Iowa City, Iowa 52242
Phone:  319-353-8587

From: Bill Lorensen <bill.lorensen at gmail.com<mailto:bill.lorensen at gmail.com>>
Date: Mon, 15 Aug 2011 11:16:37 -0400
To: David Cole <david.cole at kitware.com<mailto:david.cole at kitware.com>>
Cc: ITK <insight-developers at itk.org<mailto:insight-developers at itk.org>>
Subject: Re: [Insight-developers] Failed to merge a gerrit patch

Many of us merge another person's topic. Sometimes for someone without permission but mostly to get it there since it has been approved. It is more efficient.

I realize the workflow can change. I just think that this is a step backwards.

I guess I can always make my own alias to do what I want to do.

Bill

On Mon, Aug 15, 2011 at 11:13 AM, David Cole <david.cole at kitware.com<mailto:david.cole at kitware.com>> wrote:
Perhaps the basic problem here is that you are trying to merge someone
else's topic. Is this just to "get it in there" or is this usually
from somebody without write access...?

The author of a commit should be the one to merge it if he has write access.

I think the safeguards put in to prevent accidental merges should be
kept. Even if it forces some of us to change our workflow.

We do not have a backwards compatibility requirement on our
workflows... We do have a strong obligation to keep unintended changes
out of the 'master' branch of the main repository.

On Mon, Aug 15, 2011 at 7:58 AM, Bill Lorensen <bill.lorensen at gmail.com<mailto:bill.lorensen at gmail.com>> wrote:
> Matt,
>
> I understand your description of how things work.
>
> Before your changes to the gerrit aliases I would:
>
> 1) git checkout -b topic
> 2) cherry pick the topic
> 3) git gerrit-merge
>
> This no longer works.
>
> I think it's a bug. Try it yourself the next time you want to merge some
> else's topic (i.e. one that you have never pushed to gerrit).
>
> Bill
>
> On Sun, Aug 14, 2011 at 10:38 PM, Matthew McCormick (thewtex)
> <matt at mmmccormick.com<mailto:matt at mmmccormick.com>> wrote:
>>
>> On Sun, Aug 14, 2011 at 5:55 PM, Bill Lorensen <bill.lorensen at gmail.com<mailto:bill.lorensen at gmail.com>>
>> wrote:
>> > I cannot do a push after the cherry pick because gerrit reports that
>> > nothing
>> > has changed.
>>
>> 'git cherry-pick' says take a commit and merges it into the current
>> branch.  If the HEAD of the current branch is the same as the parent
>> of the commit, then nothings changes.  No 'git gerrit-push' is
>> required.  If, however, master is advanced ahead of parent of the
>> commit, a merge is required.  There may or may not be merge conflicts.
>>  The cherry-picked commit has a different parent, and the code that
>> surrounds it is different.  In this case, a 'git gerrit-push' is
>> required, as it should be.  It is effectively re-basing on master.
>>
>> >
>> > I can do a commit -amend and add a bogus change to the commit message
>> > and
>> > then successfully push.
>>
>> This was not necessary in this case.  A direct 'git gerrit-merge'
>> should have worked.
>>
>> > I can live with this, but it is a change from the
>> > previous work flow. I avoid the checkout line because it often rebuilds
>> > too
>> > much stuff.
>>
>> I think it is good to know when a rebase occurs, and to have builds to
>> verify that the rebase did not cause problems.  The robots are
>> computers, not people -- they will not complain to much if they have a
>> little extra work ;-).
>>
>> >
>> > Actually, I would prefer that the recent commit check be removed. I
>> > realize
>> > that it was adeed to prevent a bogur gerrit-merge, but I question how
>> > often
>> > that happened.
>>
>> I am of the mind "It's a feature, not a bug."
>>
>> Matt
>
>
> _______________________________________________
> Powered by www.kitware.com<http://www.kitware.com>
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>
>

_______________________________________________ Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Kitware offers ITK Training Courses, for more information visit: http://kitware.com/products/protraining.html Please keep messages on-topic and check the ITK FAQ at: http://www.itk.org/Wiki/ITK_FAQ Follow this link to subscribe/unsubscribe: http://www.itk.org/mailman/listinfo/insight-developers

________________________________
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110815/d93687b1/attachment.htm>