You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@libcloud.apache.org by anthony shaw <an...@gmail.com> on 2016/10/11 22:49:36 UTC

[dev] Merge process

Hi,

Our PR process (applies to committers but anyone else is welcome to
weigh in) says to download the patch file from GitHub and apply the
patch using the `git am` command.

I find git am to be so fragile, typically I use the --3way flag to
help it try and resolve conflicts but normally is just stumbles on the
slightest issue.

The new process I've been using is :

git fetch https://github.com/<remote user>/libcloud <remote-branch>:github-<pr>
git merge <github-pr>

.. edit merge message to included Closes #PR

Then push to apache trunk.

An obvious advantage is that in GitHub the PRs show as merged.
https://github.com/apache/libcloud/pull/899

The merge tool in git (instead of the patch) is so much more reliable.

What do people think of this approach? Here is an example -
https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b1408437b07563

Ant

Re: [dev] Merge process

Posted by Eric Johnson <er...@apache.org>.
Yup. Creature of habit and back in the day Tomaz preferred single commits.

On Thu, Oct 13, 2016 at 2:49 PM, anthony shaw <an...@gmail.com>
wrote:

> My question was more why do you need to rebase at all? Just to squash
> the commits for the PR?
>
> On Fri, Oct 14, 2016 at 8:48 AM, Eric Johnson
> <er...@google.com.invalid> wrote:
> > Just a creature of habit and that was how I learned to squash.
> >
> > On Thu, Oct 13, 2016 at 2:46 PM, anthony shaw <an...@gmail.com>
> > wrote:
> >
> >> ok. Now I'm curious why you have to do an interactive rebase in the
> >> first place? That tool is kinda playing with fire unless you're
> >> working off a feature branch
> >>
> >> On Fri, Oct 14, 2016 at 8:43 AM, Eric Johnson
> >> <er...@google.com.invalid> wrote:
> >> > No, on rebase, your commit just disappeared!
> >> >
> >> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <
> anthony.p.shaw@gmail.com>
> >> > wrote:
> >> >
> >> >> "hard time merging"? let me guess, "patch does not apply"? This is my
> >> >> favourite error, so much so it's like a close family member.
> >> >>
> >> >>
> >> >> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org>
> >> wrote:
> >> >> > Yup, I kicked the can down the road. My next merge for #901 had the
> >> same
> >> >> > issue.
> >> >> >
> >> >> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <erjohnso@apache.org
> >
> >> >> wrote:
> >> >> >
> >> >> >> Not sure if this related, but I had a hard time merging #856 in
> this
> >> >> >> morning.  I was following my normal procedure using git-am,
> updating
> >> >> >> CHANGES.rst, then rebasing to squash into a single commit. Prior
> to
> >> >> rebase,
> >> >> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log.
> >> During
> >> >> >> rebase -i, I wouldn't see that commit in the list and if I
> proceeded
> >> >> with
> >> >> >> my squash, that commit would get dropped.
> >> >> >>
> >> >> >> So, I either made the problem worse by not rebasing and pushing
> two
> >> >> >> commits (one for #856 and one for updating changes), or I just
> kicked
> >> >> the
> >> >> >> can down the road.  But maybe it'll be "fixed" for next committer?
> >> >> >>
> >> >> >> My git-foo isn't super strong and I'd welcome insight into how I
> >> >> could've
> >> >> >> cleaned it up with git commands.
> >> >> >>
> >> >> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org>
> >> wrote:
> >> >> >>
> >> >> >>> I personally used all in the past (am, merge, apply-patch),
> >> depending
> >> >> on
> >> >> >>> the scenario of which one was easier to work with / apply (I a
> lot
> >> of
> >> >> >>> times
> >> >> >>> I also need to check out the original branch and do some merge
> foo
> >> so I
> >> >> >>> can
> >> >> >>> merge it cleanly into trunk).
> >> >> >>>
> >> >> >>> I do prefer am since it doesn't result in a merge commit which
> makes
> >> >> the
> >> >> >>> history look slightly nicer.
> >> >> >>>
> >> >> >>> Having said that, I'm fine with whatever approach is the easier
> to
> >> >> manage
> >> >> >>> for the person applying the patch as long as it meets this
> criteria:
> >> >> >>>
> >> >> >>> - Preserve original commit author (preserve original commits as
> the
> >> >> are)
> >> >> >>> - Commit(s) are signed off by the person applying the changes
> >> >> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
> >> >> commit(s)
> >> >> >>> message
> >> >> >>>
> >> >> >>> Another option also is to try "git merge --no-commit" / "git
> merge
> >> >> >>> --squash", but we need to be careful with those so we don't
> rewrite
> >> >> >>> history
> >> >> >>> (apache git repo actually doesn't allow pushing that, but it can
> >> still
> >> >> be
> >> >> >>> annoying).
> >> >> >>>
> >> >> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
> >> >> anthony.p.shaw@gmail.com>
> >> >> >>> wrote:
> >> >> >>>
> >> >> >>> > Hi,
> >> >> >>> >
> >> >> >>> > Our PR process (applies to committers but anyone else is
> welcome
> >> to
> >> >> >>> > weigh in) says to download the patch file from GitHub and apply
> >> the
> >> >> >>> > patch using the `git am` command.
> >> >> >>> >
> >> >> >>> > I find git am to be so fragile, typically I use the --3way
> flag to
> >> >> >>> > help it try and resolve conflicts but normally is just
> stumbles on
> >> >> the
> >> >> >>> > slightest issue.
> >> >> >>> >
> >> >> >>> > The new process I've been using is :
> >> >> >>> >
> >> >> >>> > git fetch https://github.com/<remote user>/libcloud
> >> >> >>> > <remote-branch>:github-<pr>
> >> >> >>> > git merge <github-pr>
> >> >> >>> >
> >> >> >>> > .. edit merge message to included Closes #PR
> >> >> >>> >
> >> >> >>> > Then push to apache trunk.
> >> >> >>> >
> >> >> >>> > An obvious advantage is that in GitHub the PRs show as merged.
> >> >> >>> > https://github.com/apache/libcloud/pull/899
> >> >> >>> >
> >> >> >>> > The merge tool in git (instead of the patch) is so much more
> >> >> reliable.
> >> >> >>> >
> >> >> >>> > What do people think of this approach? Here is an example -
> >> >> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
> >> >> >>> 92af6220b140
> >> >> >>> > 8437b07563
> >> >> >>> >
> >> >> >>> > Ant
> >> >> >>> >
> >> >> >>>
> >> >> >>
> >> >> >>
> >> >>
> >>
>

Re: [dev] Merge process

Posted by anthony shaw <an...@gmail.com>.
My question was more why do you need to rebase at all? Just to squash
the commits for the PR?

On Fri, Oct 14, 2016 at 8:48 AM, Eric Johnson
<er...@google.com.invalid> wrote:
> Just a creature of habit and that was how I learned to squash.
>
> On Thu, Oct 13, 2016 at 2:46 PM, anthony shaw <an...@gmail.com>
> wrote:
>
>> ok. Now I'm curious why you have to do an interactive rebase in the
>> first place? That tool is kinda playing with fire unless you're
>> working off a feature branch
>>
>> On Fri, Oct 14, 2016 at 8:43 AM, Eric Johnson
>> <er...@google.com.invalid> wrote:
>> > No, on rebase, your commit just disappeared!
>> >
>> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <an...@gmail.com>
>> > wrote:
>> >
>> >> "hard time merging"? let me guess, "patch does not apply"? This is my
>> >> favourite error, so much so it's like a close family member.
>> >>
>> >>
>> >> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org>
>> wrote:
>> >> > Yup, I kicked the can down the road. My next merge for #901 had the
>> same
>> >> > issue.
>> >> >
>> >> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org>
>> >> wrote:
>> >> >
>> >> >> Not sure if this related, but I had a hard time merging #856 in this
>> >> >> morning.  I was following my normal procedure using git-am, updating
>> >> >> CHANGES.rst, then rebasing to squash into a single commit. Prior to
>> >> rebase,
>> >> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log.
>> During
>> >> >> rebase -i, I wouldn't see that commit in the list and if I proceeded
>> >> with
>> >> >> my squash, that commit would get dropped.
>> >> >>
>> >> >> So, I either made the problem worse by not rebasing and pushing two
>> >> >> commits (one for #856 and one for updating changes), or I just kicked
>> >> the
>> >> >> can down the road.  But maybe it'll be "fixed" for next committer?
>> >> >>
>> >> >> My git-foo isn't super strong and I'd welcome insight into how I
>> >> could've
>> >> >> cleaned it up with git commands.
>> >> >>
>> >> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org>
>> wrote:
>> >> >>
>> >> >>> I personally used all in the past (am, merge, apply-patch),
>> depending
>> >> on
>> >> >>> the scenario of which one was easier to work with / apply (I a lot
>> of
>> >> >>> times
>> >> >>> I also need to check out the original branch and do some merge foo
>> so I
>> >> >>> can
>> >> >>> merge it cleanly into trunk).
>> >> >>>
>> >> >>> I do prefer am since it doesn't result in a merge commit which makes
>> >> the
>> >> >>> history look slightly nicer.
>> >> >>>
>> >> >>> Having said that, I'm fine with whatever approach is the easier to
>> >> manage
>> >> >>> for the person applying the patch as long as it meets this criteria:
>> >> >>>
>> >> >>> - Preserve original commit author (preserve original commits as the
>> >> are)
>> >> >>> - Commit(s) are signed off by the person applying the changes
>> >> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
>> >> commit(s)
>> >> >>> message
>> >> >>>
>> >> >>> Another option also is to try "git merge --no-commit" / "git merge
>> >> >>> --squash", but we need to be careful with those so we don't rewrite
>> >> >>> history
>> >> >>> (apache git repo actually doesn't allow pushing that, but it can
>> still
>> >> be
>> >> >>> annoying).
>> >> >>>
>> >> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
>> >> anthony.p.shaw@gmail.com>
>> >> >>> wrote:
>> >> >>>
>> >> >>> > Hi,
>> >> >>> >
>> >> >>> > Our PR process (applies to committers but anyone else is welcome
>> to
>> >> >>> > weigh in) says to download the patch file from GitHub and apply
>> the
>> >> >>> > patch using the `git am` command.
>> >> >>> >
>> >> >>> > I find git am to be so fragile, typically I use the --3way flag to
>> >> >>> > help it try and resolve conflicts but normally is just stumbles on
>> >> the
>> >> >>> > slightest issue.
>> >> >>> >
>> >> >>> > The new process I've been using is :
>> >> >>> >
>> >> >>> > git fetch https://github.com/<remote user>/libcloud
>> >> >>> > <remote-branch>:github-<pr>
>> >> >>> > git merge <github-pr>
>> >> >>> >
>> >> >>> > .. edit merge message to included Closes #PR
>> >> >>> >
>> >> >>> > Then push to apache trunk.
>> >> >>> >
>> >> >>> > An obvious advantage is that in GitHub the PRs show as merged.
>> >> >>> > https://github.com/apache/libcloud/pull/899
>> >> >>> >
>> >> >>> > The merge tool in git (instead of the patch) is so much more
>> >> reliable.
>> >> >>> >
>> >> >>> > What do people think of this approach? Here is an example -
>> >> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
>> >> >>> 92af6220b140
>> >> >>> > 8437b07563
>> >> >>> >
>> >> >>> > Ant
>> >> >>> >
>> >> >>>
>> >> >>
>> >> >>
>> >>
>>

Re: [dev] Merge process

Posted by Eric Johnson <er...@google.com.INVALID>.
Just a creature of habit and that was how I learned to squash.

On Thu, Oct 13, 2016 at 2:46 PM, anthony shaw <an...@gmail.com>
wrote:

> ok. Now I'm curious why you have to do an interactive rebase in the
> first place? That tool is kinda playing with fire unless you're
> working off a feature branch
>
> On Fri, Oct 14, 2016 at 8:43 AM, Eric Johnson
> <er...@google.com.invalid> wrote:
> > No, on rebase, your commit just disappeared!
> >
> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <an...@gmail.com>
> > wrote:
> >
> >> "hard time merging"? let me guess, "patch does not apply"? This is my
> >> favourite error, so much so it's like a close family member.
> >>
> >>
> >> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org>
> wrote:
> >> > Yup, I kicked the can down the road. My next merge for #901 had the
> same
> >> > issue.
> >> >
> >> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org>
> >> wrote:
> >> >
> >> >> Not sure if this related, but I had a hard time merging #856 in this
> >> >> morning.  I was following my normal procedure using git-am, updating
> >> >> CHANGES.rst, then rebasing to squash into a single commit. Prior to
> >> rebase,
> >> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log.
> During
> >> >> rebase -i, I wouldn't see that commit in the list and if I proceeded
> >> with
> >> >> my squash, that commit would get dropped.
> >> >>
> >> >> So, I either made the problem worse by not rebasing and pushing two
> >> >> commits (one for #856 and one for updating changes), or I just kicked
> >> the
> >> >> can down the road.  But maybe it'll be "fixed" for next committer?
> >> >>
> >> >> My git-foo isn't super strong and I'd welcome insight into how I
> >> could've
> >> >> cleaned it up with git commands.
> >> >>
> >> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org>
> wrote:
> >> >>
> >> >>> I personally used all in the past (am, merge, apply-patch),
> depending
> >> on
> >> >>> the scenario of which one was easier to work with / apply (I a lot
> of
> >> >>> times
> >> >>> I also need to check out the original branch and do some merge foo
> so I
> >> >>> can
> >> >>> merge it cleanly into trunk).
> >> >>>
> >> >>> I do prefer am since it doesn't result in a merge commit which makes
> >> the
> >> >>> history look slightly nicer.
> >> >>>
> >> >>> Having said that, I'm fine with whatever approach is the easier to
> >> manage
> >> >>> for the person applying the patch as long as it meets this criteria:
> >> >>>
> >> >>> - Preserve original commit author (preserve original commits as the
> >> are)
> >> >>> - Commit(s) are signed off by the person applying the changes
> >> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
> >> commit(s)
> >> >>> message
> >> >>>
> >> >>> Another option also is to try "git merge --no-commit" / "git merge
> >> >>> --squash", but we need to be careful with those so we don't rewrite
> >> >>> history
> >> >>> (apache git repo actually doesn't allow pushing that, but it can
> still
> >> be
> >> >>> annoying).
> >> >>>
> >> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
> >> anthony.p.shaw@gmail.com>
> >> >>> wrote:
> >> >>>
> >> >>> > Hi,
> >> >>> >
> >> >>> > Our PR process (applies to committers but anyone else is welcome
> to
> >> >>> > weigh in) says to download the patch file from GitHub and apply
> the
> >> >>> > patch using the `git am` command.
> >> >>> >
> >> >>> > I find git am to be so fragile, typically I use the --3way flag to
> >> >>> > help it try and resolve conflicts but normally is just stumbles on
> >> the
> >> >>> > slightest issue.
> >> >>> >
> >> >>> > The new process I've been using is :
> >> >>> >
> >> >>> > git fetch https://github.com/<remote user>/libcloud
> >> >>> > <remote-branch>:github-<pr>
> >> >>> > git merge <github-pr>
> >> >>> >
> >> >>> > .. edit merge message to included Closes #PR
> >> >>> >
> >> >>> > Then push to apache trunk.
> >> >>> >
> >> >>> > An obvious advantage is that in GitHub the PRs show as merged.
> >> >>> > https://github.com/apache/libcloud/pull/899
> >> >>> >
> >> >>> > The merge tool in git (instead of the patch) is so much more
> >> reliable.
> >> >>> >
> >> >>> > What do people think of this approach? Here is an example -
> >> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
> >> >>> 92af6220b140
> >> >>> > 8437b07563
> >> >>> >
> >> >>> > Ant
> >> >>> >
> >> >>>
> >> >>
> >> >>
> >>
>

Re: [dev] Merge process

Posted by Allard Hoeve <al...@gmail.com>.
I always interactively rebase. Gives me more control, never fails me.

Empty rebase mostly means the changes are already applied through some
other means.

Best,

Allard


On Thu, Oct 13, 2016, 23:46 anthony shaw <an...@gmail.com> wrote:

> ok. Now I'm curious why you have to do an interactive rebase in the
> first place? That tool is kinda playing with fire unless you're
> working off a feature branch
>
> On Fri, Oct 14, 2016 at 8:43 AM, Eric Johnson
> <er...@google.com.invalid> wrote:
> > No, on rebase, your commit just disappeared!
> >
> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <an...@gmail.com>
> > wrote:
> >
> >> "hard time merging"? let me guess, "patch does not apply"? This is my
> >> favourite error, so much so it's like a close family member.
> >>
> >>
> >> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org>
> wrote:
> >> > Yup, I kicked the can down the road. My next merge for #901 had the
> same
> >> > issue.
> >> >
> >> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org>
> >> wrote:
> >> >
> >> >> Not sure if this related, but I had a hard time merging #856 in this
> >> >> morning.  I was following my normal procedure using git-am, updating
> >> >> CHANGES.rst, then rebasing to squash into a single commit. Prior to
> >> rebase,
> >> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log.
> During
> >> >> rebase -i, I wouldn't see that commit in the list and if I proceeded
> >> with
> >> >> my squash, that commit would get dropped.
> >> >>
> >> >> So, I either made the problem worse by not rebasing and pushing two
> >> >> commits (one for #856 and one for updating changes), or I just kicked
> >> the
> >> >> can down the road.  But maybe it'll be "fixed" for next committer?
> >> >>
> >> >> My git-foo isn't super strong and I'd welcome insight into how I
> >> could've
> >> >> cleaned it up with git commands.
> >> >>
> >> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org>
> wrote:
> >> >>
> >> >>> I personally used all in the past (am, merge, apply-patch),
> depending
> >> on
> >> >>> the scenario of which one was easier to work with / apply (I a lot
> of
> >> >>> times
> >> >>> I also need to check out the original branch and do some merge foo
> so I
> >> >>> can
> >> >>> merge it cleanly into trunk).
> >> >>>
> >> >>> I do prefer am since it doesn't result in a merge commit which makes
> >> the
> >> >>> history look slightly nicer.
> >> >>>
> >> >>> Having said that, I'm fine with whatever approach is the easier to
> >> manage
> >> >>> for the person applying the patch as long as it meets this criteria:
> >> >>>
> >> >>> - Preserve original commit author (preserve original commits as the
> >> are)
> >> >>> - Commit(s) are signed off by the person applying the changes
> >> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
> >> commit(s)
> >> >>> message
> >> >>>
> >> >>> Another option also is to try "git merge --no-commit" / "git merge
> >> >>> --squash", but we need to be careful with those so we don't rewrite
> >> >>> history
> >> >>> (apache git repo actually doesn't allow pushing that, but it can
> still
> >> be
> >> >>> annoying).
> >> >>>
> >> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
> >> anthony.p.shaw@gmail.com>
> >> >>> wrote:
> >> >>>
> >> >>> > Hi,
> >> >>> >
> >> >>> > Our PR process (applies to committers but anyone else is welcome
> to
> >> >>> > weigh in) says to download the patch file from GitHub and apply
> the
> >> >>> > patch using the `git am` command.
> >> >>> >
> >> >>> > I find git am to be so fragile, typically I use the --3way flag to
> >> >>> > help it try and resolve conflicts but normally is just stumbles on
> >> the
> >> >>> > slightest issue.
> >> >>> >
> >> >>> > The new process I've been using is :
> >> >>> >
> >> >>> > git fetch https://github.com/<remote user>/libcloud
> >> >>> > <remote-branch>:github-<pr>
> >> >>> > git merge <github-pr>
> >> >>> >
> >> >>> > .. edit merge message to included Closes #PR
> >> >>> >
> >> >>> > Then push to apache trunk.
> >> >>> >
> >> >>> > An obvious advantage is that in GitHub the PRs show as merged.
> >> >>> > https://github.com/apache/libcloud/pull/899
> >> >>> >
> >> >>> > The merge tool in git (instead of the patch) is so much more
> >> reliable.
> >> >>> >
> >> >>> > What do people think of this approach? Here is an example -
> >> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
> >> >>> 92af6220b140
> >> >>> > 8437b07563
> >> >>> >
> >> >>> > Ant
> >> >>> >
> >> >>>
> >> >>
> >> >>
> >>
>

Re: [dev] Merge process

Posted by anthony shaw <an...@gmail.com>.
ok. Now I'm curious why you have to do an interactive rebase in the
first place? That tool is kinda playing with fire unless you're
working off a feature branch

On Fri, Oct 14, 2016 at 8:43 AM, Eric Johnson
<er...@google.com.invalid> wrote:
> No, on rebase, your commit just disappeared!
>
> On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <an...@gmail.com>
> wrote:
>
>> "hard time merging"? let me guess, "patch does not apply"? This is my
>> favourite error, so much so it's like a close family member.
>>
>>
>> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org> wrote:
>> > Yup, I kicked the can down the road. My next merge for #901 had the same
>> > issue.
>> >
>> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org>
>> wrote:
>> >
>> >> Not sure if this related, but I had a hard time merging #856 in this
>> >> morning.  I was following my normal procedure using git-am, updating
>> >> CHANGES.rst, then rebasing to squash into a single commit. Prior to
>> rebase,
>> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log. During
>> >> rebase -i, I wouldn't see that commit in the list and if I proceeded
>> with
>> >> my squash, that commit would get dropped.
>> >>
>> >> So, I either made the problem worse by not rebasing and pushing two
>> >> commits (one for #856 and one for updating changes), or I just kicked
>> the
>> >> can down the road.  But maybe it'll be "fixed" for next committer?
>> >>
>> >> My git-foo isn't super strong and I'd welcome insight into how I
>> could've
>> >> cleaned it up with git commands.
>> >>
>> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org> wrote:
>> >>
>> >>> I personally used all in the past (am, merge, apply-patch), depending
>> on
>> >>> the scenario of which one was easier to work with / apply (I a lot of
>> >>> times
>> >>> I also need to check out the original branch and do some merge foo so I
>> >>> can
>> >>> merge it cleanly into trunk).
>> >>>
>> >>> I do prefer am since it doesn't result in a merge commit which makes
>> the
>> >>> history look slightly nicer.
>> >>>
>> >>> Having said that, I'm fine with whatever approach is the easier to
>> manage
>> >>> for the person applying the patch as long as it meets this criteria:
>> >>>
>> >>> - Preserve original commit author (preserve original commits as the
>> are)
>> >>> - Commit(s) are signed off by the person applying the changes
>> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
>> commit(s)
>> >>> message
>> >>>
>> >>> Another option also is to try "git merge --no-commit" / "git merge
>> >>> --squash", but we need to be careful with those so we don't rewrite
>> >>> history
>> >>> (apache git repo actually doesn't allow pushing that, but it can still
>> be
>> >>> annoying).
>> >>>
>> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
>> anthony.p.shaw@gmail.com>
>> >>> wrote:
>> >>>
>> >>> > Hi,
>> >>> >
>> >>> > Our PR process (applies to committers but anyone else is welcome to
>> >>> > weigh in) says to download the patch file from GitHub and apply the
>> >>> > patch using the `git am` command.
>> >>> >
>> >>> > I find git am to be so fragile, typically I use the --3way flag to
>> >>> > help it try and resolve conflicts but normally is just stumbles on
>> the
>> >>> > slightest issue.
>> >>> >
>> >>> > The new process I've been using is :
>> >>> >
>> >>> > git fetch https://github.com/<remote user>/libcloud
>> >>> > <remote-branch>:github-<pr>
>> >>> > git merge <github-pr>
>> >>> >
>> >>> > .. edit merge message to included Closes #PR
>> >>> >
>> >>> > Then push to apache trunk.
>> >>> >
>> >>> > An obvious advantage is that in GitHub the PRs show as merged.
>> >>> > https://github.com/apache/libcloud/pull/899
>> >>> >
>> >>> > The merge tool in git (instead of the patch) is so much more
>> reliable.
>> >>> >
>> >>> > What do people think of this approach? Here is an example -
>> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
>> >>> 92af6220b140
>> >>> > 8437b07563
>> >>> >
>> >>> > Ant
>> >>> >
>> >>>
>> >>
>> >>
>>

Re: [dev] Merge process

Posted by Eric Johnson <er...@google.com.INVALID>.
No, on rebase, your commit just disappeared!

On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <an...@gmail.com>
wrote:

> "hard time merging"? let me guess, "patch does not apply"? This is my
> favourite error, so much so it's like a close family member.
>
>
> On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org> wrote:
> > Yup, I kicked the can down the road. My next merge for #901 had the same
> > issue.
> >
> > On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org>
> wrote:
> >
> >> Not sure if this related, but I had a hard time merging #856 in this
> >> morning.  I was following my normal procedure using git-am, updating
> >> CHANGES.rst, then rebasing to squash into a single commit. Prior to
> rebase,
> >> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log. During
> >> rebase -i, I wouldn't see that commit in the list and if I proceeded
> with
> >> my squash, that commit would get dropped.
> >>
> >> So, I either made the problem worse by not rebasing and pushing two
> >> commits (one for #856 and one for updating changes), or I just kicked
> the
> >> can down the road.  But maybe it'll be "fixed" for next committer?
> >>
> >> My git-foo isn't super strong and I'd welcome insight into how I
> could've
> >> cleaned it up with git commands.
> >>
> >> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org> wrote:
> >>
> >>> I personally used all in the past (am, merge, apply-patch), depending
> on
> >>> the scenario of which one was easier to work with / apply (I a lot of
> >>> times
> >>> I also need to check out the original branch and do some merge foo so I
> >>> can
> >>> merge it cleanly into trunk).
> >>>
> >>> I do prefer am since it doesn't result in a merge commit which makes
> the
> >>> history look slightly nicer.
> >>>
> >>> Having said that, I'm fine with whatever approach is the easier to
> manage
> >>> for the person applying the patch as long as it meets this criteria:
> >>>
> >>> - Preserve original commit author (preserve original commits as the
> are)
> >>> - Commit(s) are signed off by the person applying the changes
> >>> - We can easily add "Closed #PRNUMBER" or similar message to the
> commit(s)
> >>> message
> >>>
> >>> Another option also is to try "git merge --no-commit" / "git merge
> >>> --squash", but we need to be careful with those so we don't rewrite
> >>> history
> >>> (apache git repo actually doesn't allow pushing that, but it can still
> be
> >>> annoying).
> >>>
> >>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <
> anthony.p.shaw@gmail.com>
> >>> wrote:
> >>>
> >>> > Hi,
> >>> >
> >>> > Our PR process (applies to committers but anyone else is welcome to
> >>> > weigh in) says to download the patch file from GitHub and apply the
> >>> > patch using the `git am` command.
> >>> >
> >>> > I find git am to be so fragile, typically I use the --3way flag to
> >>> > help it try and resolve conflicts but normally is just stumbles on
> the
> >>> > slightest issue.
> >>> >
> >>> > The new process I've been using is :
> >>> >
> >>> > git fetch https://github.com/<remote user>/libcloud
> >>> > <remote-branch>:github-<pr>
> >>> > git merge <github-pr>
> >>> >
> >>> > .. edit merge message to included Closes #PR
> >>> >
> >>> > Then push to apache trunk.
> >>> >
> >>> > An obvious advantage is that in GitHub the PRs show as merged.
> >>> > https://github.com/apache/libcloud/pull/899
> >>> >
> >>> > The merge tool in git (instead of the patch) is so much more
> reliable.
> >>> >
> >>> > What do people think of this approach? Here is an example -
> >>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
> >>> 92af6220b140
> >>> > 8437b07563
> >>> >
> >>> > Ant
> >>> >
> >>>
> >>
> >>
>

Re: [dev] Merge process

Posted by anthony shaw <an...@gmail.com>.
"hard time merging"? let me guess, "patch does not apply"? This is my
favourite error, so much so it's like a close family member.


On Fri, Oct 14, 2016 at 2:42 AM, Eric Johnson <er...@apache.org> wrote:
> Yup, I kicked the can down the road. My next merge for #901 had the same
> issue.
>
> On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org> wrote:
>
>> Not sure if this related, but I had a hard time merging #856 in this
>> morning.  I was following my normal procedure using git-am, updating
>> CHANGES.rst, then rebasing to squash into a single commit. Prior to rebase,
>> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log. During
>> rebase -i, I wouldn't see that commit in the list and if I proceeded with
>> my squash, that commit would get dropped.
>>
>> So, I either made the problem worse by not rebasing and pushing two
>> commits (one for #856 and one for updating changes), or I just kicked the
>> can down the road.  But maybe it'll be "fixed" for next committer?
>>
>> My git-foo isn't super strong and I'd welcome insight into how I could've
>> cleaned it up with git commands.
>>
>> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org> wrote:
>>
>>> I personally used all in the past (am, merge, apply-patch), depending on
>>> the scenario of which one was easier to work with / apply (I a lot of
>>> times
>>> I also need to check out the original branch and do some merge foo so I
>>> can
>>> merge it cleanly into trunk).
>>>
>>> I do prefer am since it doesn't result in a merge commit which makes the
>>> history look slightly nicer.
>>>
>>> Having said that, I'm fine with whatever approach is the easier to manage
>>> for the person applying the patch as long as it meets this criteria:
>>>
>>> - Preserve original commit author (preserve original commits as the are)
>>> - Commit(s) are signed off by the person applying the changes
>>> - We can easily add "Closed #PRNUMBER" or similar message to the commit(s)
>>> message
>>>
>>> Another option also is to try "git merge --no-commit" / "git merge
>>> --squash", but we need to be careful with those so we don't rewrite
>>> history
>>> (apache git repo actually doesn't allow pushing that, but it can still be
>>> annoying).
>>>
>>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
>>> wrote:
>>>
>>> > Hi,
>>> >
>>> > Our PR process (applies to committers but anyone else is welcome to
>>> > weigh in) says to download the patch file from GitHub and apply the
>>> > patch using the `git am` command.
>>> >
>>> > I find git am to be so fragile, typically I use the --3way flag to
>>> > help it try and resolve conflicts but normally is just stumbles on the
>>> > slightest issue.
>>> >
>>> > The new process I've been using is :
>>> >
>>> > git fetch https://github.com/<remote user>/libcloud
>>> > <remote-branch>:github-<pr>
>>> > git merge <github-pr>
>>> >
>>> > .. edit merge message to included Closes #PR
>>> >
>>> > Then push to apache trunk.
>>> >
>>> > An obvious advantage is that in GitHub the PRs show as merged.
>>> > https://github.com/apache/libcloud/pull/899
>>> >
>>> > The merge tool in git (instead of the patch) is so much more reliable.
>>> >
>>> > What do people think of this approach? Here is an example -
>>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
>>> 92af6220b140
>>> > 8437b07563
>>> >
>>> > Ant
>>> >
>>>
>>
>>

Re: [dev] Merge process

Posted by Eric Johnson <er...@apache.org>.
Yup, I kicked the can down the road. My next merge for #901 had the same
issue.

On Thu, Oct 13, 2016 at 8:19 AM, Eric Johnson <er...@apache.org> wrote:

> Not sure if this related, but I had a hard time merging #856 in this
> morning.  I was following my normal procedure using git-am, updating
> CHANGES.rst, then rebasing to squash into a single commit. Prior to rebase,
> I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log. During
> rebase -i, I wouldn't see that commit in the list and if I proceeded with
> my squash, that commit would get dropped.
>
> So, I either made the problem worse by not rebasing and pushing two
> commits (one for #856 and one for updating changes), or I just kicked the
> can down the road.  But maybe it'll be "fixed" for next committer?
>
> My git-foo isn't super strong and I'd welcome insight into how I could've
> cleaned it up with git commands.
>
> On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org> wrote:
>
>> I personally used all in the past (am, merge, apply-patch), depending on
>> the scenario of which one was easier to work with / apply (I a lot of
>> times
>> I also need to check out the original branch and do some merge foo so I
>> can
>> merge it cleanly into trunk).
>>
>> I do prefer am since it doesn't result in a merge commit which makes the
>> history look slightly nicer.
>>
>> Having said that, I'm fine with whatever approach is the easier to manage
>> for the person applying the patch as long as it meets this criteria:
>>
>> - Preserve original commit author (preserve original commits as the are)
>> - Commit(s) are signed off by the person applying the changes
>> - We can easily add "Closed #PRNUMBER" or similar message to the commit(s)
>> message
>>
>> Another option also is to try "git merge --no-commit" / "git merge
>> --squash", but we need to be careful with those so we don't rewrite
>> history
>> (apache git repo actually doesn't allow pushing that, but it can still be
>> annoying).
>>
>> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
>> wrote:
>>
>> > Hi,
>> >
>> > Our PR process (applies to committers but anyone else is welcome to
>> > weigh in) says to download the patch file from GitHub and apply the
>> > patch using the `git am` command.
>> >
>> > I find git am to be so fragile, typically I use the --3way flag to
>> > help it try and resolve conflicts but normally is just stumbles on the
>> > slightest issue.
>> >
>> > The new process I've been using is :
>> >
>> > git fetch https://github.com/<remote user>/libcloud
>> > <remote-branch>:github-<pr>
>> > git merge <github-pr>
>> >
>> > .. edit merge message to included Closes #PR
>> >
>> > Then push to apache trunk.
>> >
>> > An obvious advantage is that in GitHub the PRs show as merged.
>> > https://github.com/apache/libcloud/pull/899
>> >
>> > The merge tool in git (instead of the patch) is so much more reliable.
>> >
>> > What do people think of this approach? Here is an example -
>> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b
>> 92af6220b140
>> > 8437b07563
>> >
>> > Ant
>> >
>>
>
>

Re: [dev] Merge process

Posted by Eric Johnson <er...@apache.org>.
Not sure if this related, but I had a hard time merging #856 in this
morning.  I was following my normal procedure using git-am, updating
CHANGES.rst, then rebasing to squash into a single commit. Prior to rebase,
I'd see 065d1919d8cd1e651b92af6220b1408437b07563 in my git-log. During
rebase -i, I wouldn't see that commit in the list and if I proceeded with
my squash, that commit would get dropped.

So, I either made the problem worse by not rebasing and pushing two commits
(one for #856 and one for updating changes), or I just kicked the can down
the road.  But maybe it'll be "fixed" for next committer?

My git-foo isn't super strong and I'd welcome insight into how I could've
cleaned it up with git commands.

On Wed, Oct 12, 2016 at 9:53 AM, Tomaz Muraus <to...@apache.org> wrote:

> I personally used all in the past (am, merge, apply-patch), depending on
> the scenario of which one was easier to work with / apply (I a lot of times
> I also need to check out the original branch and do some merge foo so I can
> merge it cleanly into trunk).
>
> I do prefer am since it doesn't result in a merge commit which makes the
> history look slightly nicer.
>
> Having said that, I'm fine with whatever approach is the easier to manage
> for the person applying the patch as long as it meets this criteria:
>
> - Preserve original commit author (preserve original commits as the are)
> - Commit(s) are signed off by the person applying the changes
> - We can easily add "Closed #PRNUMBER" or similar message to the commit(s)
> message
>
> Another option also is to try "git merge --no-commit" / "git merge
> --squash", but we need to be careful with those so we don't rewrite history
> (apache git repo actually doesn't allow pushing that, but it can still be
> annoying).
>
> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
> wrote:
>
> > Hi,
> >
> > Our PR process (applies to committers but anyone else is welcome to
> > weigh in) says to download the patch file from GitHub and apply the
> > patch using the `git am` command.
> >
> > I find git am to be so fragile, typically I use the --3way flag to
> > help it try and resolve conflicts but normally is just stumbles on the
> > slightest issue.
> >
> > The new process I've been using is :
> >
> > git fetch https://github.com/<remote user>/libcloud
> > <remote-branch>:github-<pr>
> > git merge <github-pr>
> >
> > .. edit merge message to included Closes #PR
> >
> > Then push to apache trunk.
> >
> > An obvious advantage is that in GitHub the PRs show as merged.
> > https://github.com/apache/libcloud/pull/899
> >
> > The merge tool in git (instead of the patch) is so much more reliable.
> >
> > What do people think of this approach? Here is an example -
> > https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b140
> > 8437b07563
> >
> > Ant
> >
>

Re: [dev] Merge process

Posted by Tomaz Muraus <to...@apache.org>.
I personally used all in the past (am, merge, apply-patch), depending on
the scenario of which one was easier to work with / apply (I a lot of times
I also need to check out the original branch and do some merge foo so I can
merge it cleanly into trunk).

I do prefer am since it doesn't result in a merge commit which makes the
history look slightly nicer.

Having said that, I'm fine with whatever approach is the easier to manage
for the person applying the patch as long as it meets this criteria:

- Preserve original commit author (preserve original commits as the are)
- Commit(s) are signed off by the person applying the changes
- We can easily add "Closed #PRNUMBER" or similar message to the commit(s)
message

Another option also is to try "git merge --no-commit" / "git merge
--squash", but we need to be careful with those so we don't rewrite history
(apache git repo actually doesn't allow pushing that, but it can still be
annoying).

On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
wrote:

> Hi,
>
> Our PR process (applies to committers but anyone else is welcome to
> weigh in) says to download the patch file from GitHub and apply the
> patch using the `git am` command.
>
> I find git am to be so fragile, typically I use the --3way flag to
> help it try and resolve conflicts but normally is just stumbles on the
> slightest issue.
>
> The new process I've been using is :
>
> git fetch https://github.com/<remote user>/libcloud
> <remote-branch>:github-<pr>
> git merge <github-pr>
>
> .. edit merge message to included Closes #PR
>
> Then push to apache trunk.
>
> An obvious advantage is that in GitHub the PRs show as merged.
> https://github.com/apache/libcloud/pull/899
>
> The merge tool in git (instead of the patch) is so much more reliable.
>
> What do people think of this approach? Here is an example -
> https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b140
> 8437b07563
>
> Ant
>

Re: [dev] Merge process

Posted by anthony shaw <an...@gmail.com>.
Hi Eric,

The downside is that we'll get the git commit history from the
original authors without the ("with xxx") signoff tag, but instead
that's replaced with a merge commit saying "merging GITHUB-123 by
blah", the merge commit has the parents of the original commits and as
long as we fetch the remote into a local branch named after the PR
it's also a lot easier to see which commits' were merged.
https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b1408437b07563

I know the GitHub contributor graphs explicitly hide merge commits as well.

I will see if I can script the git-commit amend command with the
signoff flag so in the history we'll also see who signed off the
commits "git commit --amend --signoff"

Anthony

On Wed, Oct 12, 2016 at 10:08 AM, Eric Johnson <er...@apache.org> wrote:
> I agree that git-am can be a pain.
>
> What I like about using it though, is that history is more forthcoming
> about original author versus us showing up all the time.
>
> """
> commit 065d1919d8cd1e651b92af6220b1408437b07563
> Merge: 4897933 0d1a9d2
> Author: Anthony Shaw <an...@apache.org>
> Date:   Wed Oct 12 09:41:47 2016 +1100
>
>     Merge branch 'libcloud-889' into trunk
>     Closes #889
> """
>
> Versus an older git-am commit entry,
>
> """
> commit f5ff0cfb080b767b542e9deec5ecc34dedcb4f0c
> Author: Fahri Cihan Demirci <fe...@users.noreply.github.com>
> Date:   Sun Oct 9 02:15:10 2016 -0400
>
>     LIBCLOUD-858: Fix Listing Libvirt Nodes with Python 3
>
>     Closes #894
> """
>
> Plus in the GitHub web UI, you can see original authors more clearly
> [scroll down to look at these same two commits,
> https://github.com/apache/libcloud/commits/trunk]
>
> If there's a way to preserve that, I'm all for stepping away from git-am.
>
> On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
> wrote:
>
>> Hi,
>>
>> Our PR process (applies to committers but anyone else is welcome to
>> weigh in) says to download the patch file from GitHub and apply the
>> patch using the `git am` command.
>>
>> I find git am to be so fragile, typically I use the --3way flag to
>> help it try and resolve conflicts but normally is just stumbles on the
>> slightest issue.
>>
>> The new process I've been using is :
>>
>> git fetch https://github.com/<remote user>/libcloud
>> <remote-branch>:github-<pr>
>> git merge <github-pr>
>>
>> .. edit merge message to included Closes #PR
>>
>> Then push to apache trunk.
>>
>> An obvious advantage is that in GitHub the PRs show as merged.
>> https://github.com/apache/libcloud/pull/899
>>
>> The merge tool in git (instead of the patch) is so much more reliable.
>>
>> What do people think of this approach? Here is an example -
>> https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b140
>> 8437b07563
>>
>> Ant
>>

Re: [dev] Merge process

Posted by Eric Johnson <er...@apache.org>.
I agree that git-am can be a pain.

What I like about using it though, is that history is more forthcoming
about original author versus us showing up all the time.

"""
commit 065d1919d8cd1e651b92af6220b1408437b07563
Merge: 4897933 0d1a9d2
Author: Anthony Shaw <an...@apache.org>
Date:   Wed Oct 12 09:41:47 2016 +1100

    Merge branch 'libcloud-889' into trunk
    Closes #889
"""

Versus an older git-am commit entry,

"""
commit f5ff0cfb080b767b542e9deec5ecc34dedcb4f0c
Author: Fahri Cihan Demirci <fe...@users.noreply.github.com>
Date:   Sun Oct 9 02:15:10 2016 -0400

    LIBCLOUD-858: Fix Listing Libvirt Nodes with Python 3

    Closes #894
"""

Plus in the GitHub web UI, you can see original authors more clearly
[scroll down to look at these same two commits,
https://github.com/apache/libcloud/commits/trunk]

If there's a way to preserve that, I'm all for stepping away from git-am.

On Tue, Oct 11, 2016 at 3:49 PM, anthony shaw <an...@gmail.com>
wrote:

> Hi,
>
> Our PR process (applies to committers but anyone else is welcome to
> weigh in) says to download the patch file from GitHub and apply the
> patch using the `git am` command.
>
> I find git am to be so fragile, typically I use the --3way flag to
> help it try and resolve conflicts but normally is just stumbles on the
> slightest issue.
>
> The new process I've been using is :
>
> git fetch https://github.com/<remote user>/libcloud
> <remote-branch>:github-<pr>
> git merge <github-pr>
>
> .. edit merge message to included Closes #PR
>
> Then push to apache trunk.
>
> An obvious advantage is that in GitHub the PRs show as merged.
> https://github.com/apache/libcloud/pull/899
>
> The merge tool in git (instead of the patch) is so much more reliable.
>
> What do people think of this approach? Here is an example -
> https://github.com/apache/libcloud/commit/065d1919d8cd1e651b92af6220b140
> 8437b07563
>
> Ant
>