Re: [dev] Merge process

2016-10-13 Thread Eric Johnson 
Yup. Creature of habit and back in the day Tomaz preferred single commits.

On Thu, Oct 13, 2016 at 2:49 PM, anthony shaw 
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
>  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 
> > 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
> >>  wrote:
> >> > No, on rebase, your commit just disappeared!
> >> >
> >> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw <
> anthony.p.s...@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 
> >> 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  >
> >> >> 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 
> >> 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.s...@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//libcloud
> >> >> >>> > :github-
> >> >> >>> > git merge 
> >> >> >>> >
> >> >> >>> > .. 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

Re: [dev] Merge process

2016-10-13 Thread anthony shaw 
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
 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 
> 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
>>  wrote:
>> > No, on rebase, your commit just disappeared!
>> >
>> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw 
>> > 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 
>> 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 
>> >> 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 
>> 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.s...@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//libcloud
>> >> >>> > :github-
>> >> >>> > git merge 
>> >> >>> >
>> >> >>> > .. 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

2016-10-13 Thread Eric Johnson 
Just a creature of habit and that was how I learned to squash.

On Thu, Oct 13, 2016 at 2:46 PM, anthony shaw 
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
>  wrote:
> > No, on rebase, your commit just disappeared!
> >
> > On Thu, Oct 13, 2016 at 2:41 PM, anthony shaw 
> > 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 
> 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 
> >> 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 
> 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.s...@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//libcloud
> >> >>> > :github-
> >> >>> > git merge 
> >> >>> >
> >> >>> > .. 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

2016-10-13 Thread anthony shaw 
"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  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  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  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 
>>> 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//libcloud
>>> > :github-
>>> > git merge 
>>> >
>>> > .. 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

2016-10-11 Thread anthony shaw 
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  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 
> 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 
> 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 
> 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//libcloud
>> :github-
>> git merge 
>>
>> .. 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
>>