You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@reef.apache.org by Brian Cho <ch...@gmail.com> on 2014/11/22 16:06:08 UTC

Adventures while merging

Hi,

I just finished merging REEF-29 / PR #15

The PR resulted in some discussion and many changes (it wasn't a trivial
PR). The changes happened as pushes to the PR branch, and simultaneously
commits were made to master.

Now, when I tried the `git rebase -i apache/master` method by picking the
first commit, and squashing the rest, the rebase did not go cleanly at all.
There were merge conflicts that I had to address, and even some merges that
happened that were plain wrong. I ended up painstakingly addressing all the
conflicts to manually match
https://github.com/apache/incubator-reef/pull/15.diff

Has anyone else had this experience? I feel something is wrong with our
method, given that github is able to find the correct diff (the link above)
without conflicts, but our method does the wrong thing. Please add to this
thread if you have any similar adventures (now or in the future).

Thanks,
Brian

Re: Adventures while merging

Posted by Byung-Gon Chun <bg...@gmail.com>.
Sounds good, Brian and Julia.

On Tue, Nov 25, 2014 at 2:43 AM, Julia Wang (QIUHE) <
Qiuhe.Wang@microsoft.com> wrote:

> Good approach!
>
> When the complex situation happens in such cases, I would think this can
> be done by the author as he/she is more familiar with what has been changed
> and can verify the change after the diff is applied. It also reduces the
> work for the committer.
>
> Thanks,
> Julia
>
> -----Original Message-----
> From: Markus Weimer [mailto:markus@weimo.de]
> Sent: Sunday, November 23, 2014 5:36 PM
> To: dev@reef.incubator.apache.org
> Subject: Re: Adventures while merging
>
> Hi,
>
> excellent, thanks! Yes, please add this as "the other way" to the comitter
> guide.
>
> Thanks,
>
> Markus
>
>
> On 2014-11-23 16:39, Brian Cho wrote:
> > Hi,
> >
> > I tested things out by checking out the previous commit before #15 was
> > applied. Using the diff works well:
> >
> > $ wget https://github.com/apache/incubator-reef/pull/15.diff
> > $ git apply 15.diff
> >
> >
> > And the author can be changed by:
> >
> > $ git commit --amend --author "New Author Name <em...@address.com>"
> >
> >
> > In cases where rebase fails, this seems like the best way to go. The
> > downside is the merger will have to write the commit message from
> > scratch, while rebase does fill in some text for you.
> >
> > Should I add this as an optional way to merge in the committer's guide?
> >
> >
> >
> > FYI, the following does _not_ work:
> >
> > $ wget https://github.com/apache/incubator-reef/pull/15.patch
> > $ git am 15.patch
> >
> >
> > .diff has the same information as "Files changed" in the GUI. .patch
> > actually lists the commit history, one by one. Running am invokes the
> > same process as rebase, as far as I can tell.
> >
> >
> > -Brian
> >
> > On Sun, Nov 23, 2014 at 2:44 PM, Markus Weimer <ma...@weimo.de> wrote:
> >
> > Yes, that is what I tried to suggest. In cases where merges are hard,
> > GitHub's diff can be used as a patch. As long as we figure out how to
> > attribute the code to the right author. I'll try that with my next merge.
> >
> > Markus
> >
> >
> > On 2014-11-22 14:03, Byung-Gon Chun wrote:
> >>>> How about applying a patch file?
> >>>>
> > https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committe
> > rs+Guide
> >>>>
> >>>>  On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de>
> >>>> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> yes, this has happened to me before. [REEF-28] / #12 was quite
> >>>> tricky to merge, in that case because it started out not as a clean
> >>>> single commit over current master. I did not have the patience to
> >>>> resolve all the merge conflicts, so I instead did the following:
> >>>>
> >>>> * Create a new branch myself. * Checkout all the changed files from
> >>>> the PR * Commit & merge that branch.
> >>>>
> >>>> That commit can be found here:
> >>>>
> >>>>
> >>>>
> > https://github.com/apache/incubator-reef/commit/141e4a321061403fc77708
> > 7207869695aedd3b84
> >>>>
> >>>>  As you can see, we lost the authorship attribution along the way,
> >>>> as the code now looks like a contribution by me, while it was
> >>>> written by Julia. I believe that could have been fixed with some
> >>>> git magic to edit the author information.
> >>>>
> >>>> This approach doesn't always work, as it looses potential changes
> >>>> to the *same* files committed. That wasn't the case here, luckily.
> >>>>
> >>>> I wonder whether a similar thing could be achieved by using the
> >>>> patch provided by GitHub. Basically, one would
> >>>>
> >>>> * Create a new branch off of master * Merge that patch in * Commit,
> >>>> merge and push
> >>>>
> >>>> That *should* work cleanly. All we need to do is find a way to fix
> >>>> the authorship attribution.
> >>>>
> >>>> Thoughts?
> >>>>
> >>>> Markus
> >>>>
> >>>>
> >>>> On 2014-11-22 07:06, Brian Cho wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> I just finished merging REEF-29 / PR #15
> >>>>>>>
> >>>>>>> The PR resulted in some discussion and many changes (it wasn't a
> >>>>>>> trivial PR). The changes happened as pushes to the PR branch,
> >>>>>>> and simultaneously commits were made to master.
> >>>>>>>
> >>>>>>> Now, when I tried the `git rebase -i apache/master` method by
> >>>>>>> picking the first commit, and squashing the rest, the rebase did
> >>>>>>> not go cleanly at all. There were merge conflicts that I had to
> >>>>>>> address, and even some merges that happened that were plain
> >>>>>>> wrong. I ended up painstakingly addressing all the conflicts to
> >>>>>>> manually match
> >>>>>>> https://github.com/apache/incubator-reef/pull/15.diff
> >>>>>>>
> >>>>>>> Has anyone else had this experience? I feel something is wrong
> >>>>>>> with our method, given that github is able to find the correct
> >>>>>>> diff (the link above) without conflicts, but our method does the
> >>>>>>> wrong thing. Please add to this thread if you have any similar
> >>>>>>> adventures (now or in the future).
> >>>>>>>
> >>>>>>> Thanks, Brian
> >>>>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>
> >
>



-- 
Byung-Gon Chun

Re: Adventures while merging

Posted by Markus Weimer <ma...@weimo.de>.
Hi,

yes, depending on who the contributor is (e.g. one of the comitters),
we can ask them to close the PR and file a new one which is a single
commit off of current master.

However, not all contributors we'd like to invite will have the needed
git-magic internalized. Hence, the work will sometimes fall onto the
comitter.

Markus


On 2014-11-24 09:43, Julia Wang (QIUHE) wrote:
> Good approach!
> 
> When the complex situation happens in such cases, I would think
> this can be done by the author as he/she is more familiar with what
> has been changed and can verify the change after the diff is
> applied. It also reduces the work for the committer.
> 
> Thanks, Julia
> 
> -----Original Message----- From: Markus Weimer
> [mailto:markus@weimo.de] Sent: Sunday, November 23, 2014 5:36 PM 
> To: dev@reef.incubator.apache.org Subject: Re: Adventures while
> merging
> 
> Hi,
> 
> excellent, thanks! Yes, please add this as "the other way" to the
> comitter guide.
> 
> Thanks,
> 
> Markus
> 
> 
> On 2014-11-23 16:39, Brian Cho wrote:
>> Hi,
>> 
>> I tested things out by checking out the previous commit before
>> #15 was applied. Using the diff works well:
>> 
>> $ wget https://github.com/apache/incubator-reef/pull/15.diff $
>> git apply 15.diff
>> 
>> 
>> And the author can be changed by:
>> 
>> $ git commit --amend --author "New Author Name
>> <em...@address.com>"
>> 
>> 
>> In cases where rebase fails, this seems like the best way to go.
>> The downside is the merger will have to write the commit message
>> from scratch, while rebase does fill in some text for you.
>> 
>> Should I add this as an optional way to merge in the committer's
>> guide?
>> 
>> 
>> 
>> FYI, the following does _not_ work:
>> 
>> $ wget https://github.com/apache/incubator-reef/pull/15.patch $
>> git am 15.patch
>> 
>> 
>> .diff has the same information as "Files changed" in the GUI.
>> .patch actually lists the commit history, one by one. Running am
>> invokes the same process as rebase, as far as I can tell.
>> 
>> 
>> -Brian
>> 
>> On Sun, Nov 23, 2014 at 2:44 PM, Markus Weimer <ma...@weimo.de>
>> wrote:
>> 
>> Yes, that is what I tried to suggest. In cases where merges are
>> hard, GitHub's diff can be used as a patch. As long as we figure
>> out how to attribute the code to the right author. I'll try that
>> with my next merge.
>> 
>> Markus
>> 
>> 
>> On 2014-11-22 14:03, Byung-Gon Chun wrote:
>>>>> How about applying a patch file?
>>>>> 
>> https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committe
>>
>> 
rs+Guide
>>>>> 
>>>>> On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer
>>>>> <ma...@weimo.de> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> yes, this has happened to me before. [REEF-28] / #12 was
>>>>> quite tricky to merge, in that case because it started out
>>>>> not as a clean single commit over current master. I did not
>>>>> have the patience to resolve all the merge conflicts, so I
>>>>> instead did the following:
>>>>> 
>>>>> * Create a new branch myself. * Checkout all the changed
>>>>> files from the PR * Commit & merge that branch.
>>>>> 
>>>>> That commit can be found here:
>>>>> 
>>>>> 
>>>>> 
>> https://github.com/apache/incubator-reef/commit/141e4a321061403fc77708
>>
>> 
7207869695aedd3b84
>>>>> 
>>>>> As you can see, we lost the authorship attribution along
>>>>> the way, as the code now looks like a contribution by me,
>>>>> while it was written by Julia. I believe that could have
>>>>> been fixed with some git magic to edit the author
>>>>> information.
>>>>> 
>>>>> This approach doesn't always work, as it looses potential
>>>>> changes to the *same* files committed. That wasn't the case
>>>>> here, luckily.
>>>>> 
>>>>> I wonder whether a similar thing could be achieved by using
>>>>> the patch provided by GitHub. Basically, one would
>>>>> 
>>>>> * Create a new branch off of master * Merge that patch in *
>>>>> Commit, merge and push
>>>>> 
>>>>> That *should* work cleanly. All we need to do is find a way
>>>>> to fix the authorship attribution.
>>>>> 
>>>>> Thoughts?
>>>>> 
>>>>> Markus
>>>>> 
>>>>> 
>>>>> On 2014-11-22 07:06, Brian Cho wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> I just finished merging REEF-29 / PR #15
>>>>>>>> 
>>>>>>>> The PR resulted in some discussion and many changes
>>>>>>>> (it wasn't a trivial PR). The changes happened as
>>>>>>>> pushes to the PR branch, and simultaneously commits
>>>>>>>> were made to master.
>>>>>>>> 
>>>>>>>> Now, when I tried the `git rebase -i apache/master`
>>>>>>>> method by picking the first commit, and squashing the
>>>>>>>> rest, the rebase did not go cleanly at all. There
>>>>>>>> were merge conflicts that I had to address, and even
>>>>>>>> some merges that happened that were plain wrong. I
>>>>>>>> ended up painstakingly addressing all the conflicts
>>>>>>>> to manually match 
>>>>>>>> https://github.com/apache/incubator-reef/pull/15.diff
>>>>>>>>
>>>>>>>>
>>>>>>>> 
Has anyone else had this experience? I feel something is wrong
>>>>>>>> with our method, given that github is able to find
>>>>>>>> the correct diff (the link above) without conflicts,
>>>>>>>> but our method does the wrong thing. Please add to
>>>>>>>> this thread if you have any similar adventures (now
>>>>>>>> or in the future).
>>>>>>>> 
>>>>>>>> Thanks, Brian
>>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>> 
>> 

RE: Adventures while merging

Posted by "Julia Wang (QIUHE)" <Qi...@microsoft.com>.
Good approach! 

When the complex situation happens in such cases, I would think this can be done by the author as he/she is more familiar with what has been changed and can verify the change after the diff is applied. It also reduces the work for the committer. 

Thanks,
Julia

-----Original Message-----
From: Markus Weimer [mailto:markus@weimo.de] 
Sent: Sunday, November 23, 2014 5:36 PM
To: dev@reef.incubator.apache.org
Subject: Re: Adventures while merging

Hi,

excellent, thanks! Yes, please add this as "the other way" to the comitter guide.

Thanks,

Markus


On 2014-11-23 16:39, Brian Cho wrote:
> Hi,
> 
> I tested things out by checking out the previous commit before #15 was 
> applied. Using the diff works well:
> 
> $ wget https://github.com/apache/incubator-reef/pull/15.diff
> $ git apply 15.diff
> 
> 
> And the author can be changed by:
> 
> $ git commit --amend --author "New Author Name <em...@address.com>"
> 
> 
> In cases where rebase fails, this seems like the best way to go. The 
> downside is the merger will have to write the commit message from 
> scratch, while rebase does fill in some text for you.
> 
> Should I add this as an optional way to merge in the committer's guide?
> 
> 
> 
> FYI, the following does _not_ work:
> 
> $ wget https://github.com/apache/incubator-reef/pull/15.patch
> $ git am 15.patch
> 
> 
> .diff has the same information as "Files changed" in the GUI. .patch 
> actually lists the commit history, one by one. Running am invokes the 
> same process as rebase, as far as I can tell.
> 
> 
> -Brian
> 
> On Sun, Nov 23, 2014 at 2:44 PM, Markus Weimer <ma...@weimo.de> wrote:
> 
> Yes, that is what I tried to suggest. In cases where merges are hard, 
> GitHub's diff can be used as a patch. As long as we figure out how to 
> attribute the code to the right author. I'll try that with my next merge.
> 
> Markus
> 
> 
> On 2014-11-22 14:03, Byung-Gon Chun wrote:
>>>> How about applying a patch file?
>>>>
> https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committe
> rs+Guide
>>>>
>>>>  On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> yes, this has happened to me before. [REEF-28] / #12 was quite 
>>>> tricky to merge, in that case because it started out not as a clean 
>>>> single commit over current master. I did not have the patience to 
>>>> resolve all the merge conflicts, so I instead did the following:
>>>>
>>>> * Create a new branch myself. * Checkout all the changed files from 
>>>> the PR * Commit & merge that branch.
>>>>
>>>> That commit can be found here:
>>>>
>>>>
>>>>
> https://github.com/apache/incubator-reef/commit/141e4a321061403fc77708
> 7207869695aedd3b84
>>>>
>>>>  As you can see, we lost the authorship attribution along the way, 
>>>> as the code now looks like a contribution by me, while it was 
>>>> written by Julia. I believe that could have been fixed with some 
>>>> git magic to edit the author information.
>>>>
>>>> This approach doesn't always work, as it looses potential changes 
>>>> to the *same* files committed. That wasn't the case here, luckily.
>>>>
>>>> I wonder whether a similar thing could be achieved by using the 
>>>> patch provided by GitHub. Basically, one would
>>>>
>>>> * Create a new branch off of master * Merge that patch in * Commit, 
>>>> merge and push
>>>>
>>>> That *should* work cleanly. All we need to do is find a way to fix 
>>>> the authorship attribution.
>>>>
>>>> Thoughts?
>>>>
>>>> Markus
>>>>
>>>>
>>>> On 2014-11-22 07:06, Brian Cho wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I just finished merging REEF-29 / PR #15
>>>>>>>
>>>>>>> The PR resulted in some discussion and many changes (it wasn't a 
>>>>>>> trivial PR). The changes happened as pushes to the PR branch, 
>>>>>>> and simultaneously commits were made to master.
>>>>>>>
>>>>>>> Now, when I tried the `git rebase -i apache/master` method by 
>>>>>>> picking the first commit, and squashing the rest, the rebase did 
>>>>>>> not go cleanly at all. There were merge conflicts that I had to 
>>>>>>> address, and even some merges that happened that were plain 
>>>>>>> wrong. I ended up painstakingly addressing all the conflicts to 
>>>>>>> manually match 
>>>>>>> https://github.com/apache/incubator-reef/pull/15.diff
>>>>>>>
>>>>>>> Has anyone else had this experience? I feel something is wrong 
>>>>>>> with our method, given that github is able to find the correct 
>>>>>>> diff (the link above) without conflicts, but our method does the 
>>>>>>> wrong thing. Please add to this thread if you have any similar 
>>>>>>> adventures (now or in the future).
>>>>>>>
>>>>>>> Thanks, Brian
>>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
> 

Re: Adventures while merging

Posted by Markus Weimer <ma...@weimo.de>.
Hi,

excellent, thanks! Yes, please add this as "the other way" to the
comitter guide.

Thanks,

Markus


On 2014-11-23 16:39, Brian Cho wrote:
> Hi,
> 
> I tested things out by checking out the previous commit before #15 was
> applied. Using the diff works well:
> 
> $ wget https://github.com/apache/incubator-reef/pull/15.diff
> $ git apply 15.diff
> 
> 
> And the author can be changed by:
> 
> $ git commit --amend --author "New Author Name <em...@address.com>"
> 
> 
> In cases where rebase fails, this seems like the best way to go. The
> downside is the merger will have to write the commit message from scratch,
> while rebase does fill in some text for you.
> 
> Should I add this as an optional way to merge in the committer's guide?
> 
> 
> 
> FYI, the following does _not_ work:
> 
> $ wget https://github.com/apache/incubator-reef/pull/15.patch
> $ git am 15.patch
> 
> 
> .diff has the same information as "Files changed" in the GUI. .patch
> actually lists the commit history, one by one. Running am invokes the same
> process as rebase, as far as I can tell.
> 
> 
> -Brian
> 
> On Sun, Nov 23, 2014 at 2:44 PM, Markus Weimer <ma...@weimo.de> wrote:
> 
> Yes, that is what I tried to suggest. In cases where merges are hard,
> GitHub's diff can be used as a patch. As long as we figure out how to
> attribute the code to the right author. I'll try that with my next merge.
> 
> Markus
> 
> 
> On 2014-11-22 14:03, Byung-Gon Chun wrote:
>>>> How about applying a patch file?
>>>>
> https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committers+Guide
>>>>
>>>>  On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> yes, this has happened to me before. [REEF-28] / #12 was quite
>>>> tricky to merge, in that case because it started out not as a clean
>>>> single commit over current master. I did not have the patience to
>>>> resolve all the merge conflicts, so I instead did the following:
>>>>
>>>> * Create a new branch myself. * Checkout all the changed files from
>>>> the PR * Commit & merge that branch.
>>>>
>>>> That commit can be found here:
>>>>
>>>>
>>>>
> https://github.com/apache/incubator-reef/commit/141e4a321061403fc777087207869695aedd3b84
>>>>
>>>>  As you can see, we lost the authorship attribution along the way,
>>>> as the code now looks like a contribution by me, while it was
>>>> written by Julia. I believe that could have been fixed with some
>>>> git magic to edit the author information.
>>>>
>>>> This approach doesn't always work, as it looses potential changes
>>>> to the *same* files committed. That wasn't the case here, luckily.
>>>>
>>>> I wonder whether a similar thing could be achieved by using the
>>>> patch provided by GitHub. Basically, one would
>>>>
>>>> * Create a new branch off of master * Merge that patch in * Commit,
>>>> merge and push
>>>>
>>>> That *should* work cleanly. All we need to do is find a way to fix
>>>> the authorship attribution.
>>>>
>>>> Thoughts?
>>>>
>>>> Markus
>>>>
>>>>
>>>> On 2014-11-22 07:06, Brian Cho wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I just finished merging REEF-29 / PR #15
>>>>>>>
>>>>>>> The PR resulted in some discussion and many changes (it
>>>>>>> wasn't a trivial PR). The changes happened as pushes to the
>>>>>>> PR branch, and simultaneously commits were made to master.
>>>>>>>
>>>>>>> Now, when I tried the `git rebase -i apache/master` method
>>>>>>> by picking the first commit, and squashing the rest, the
>>>>>>> rebase did not go cleanly at all. There were merge conflicts
>>>>>>> that I had to address, and even some merges that happened
>>>>>>> that were plain wrong. I ended up painstakingly addressing
>>>>>>> all the conflicts to manually match
>>>>>>> https://github.com/apache/incubator-reef/pull/15.diff
>>>>>>>
>>>>>>> Has anyone else had this experience? I feel something is
>>>>>>> wrong with our method, given that github is able to find the
>>>>>>> correct diff (the link above) without conflicts, but our
>>>>>>> method does the wrong thing. Please add to this thread if you
>>>>>>> have any similar adventures (now or in the future).
>>>>>>>
>>>>>>> Thanks, Brian
>>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
> 

Re: Adventures while merging

Posted by Brian Cho <ch...@gmail.com>.
Hi,

I tested things out by checking out the previous commit before #15 was
applied. Using the diff works well:

$ wget https://github.com/apache/incubator-reef/pull/15.diff
$ git apply 15.diff


And the author can be changed by:

$ git commit --amend --author "New Author Name <em...@address.com>"


In cases where rebase fails, this seems like the best way to go. The
downside is the merger will have to write the commit message from scratch,
while rebase does fill in some text for you.

Should I add this as an optional way to merge in the committer's guide?



FYI, the following does _not_ work:

$ wget https://github.com/apache/incubator-reef/pull/15.patch
$ git am 15.patch


.diff has the same information as "Files changed" in the GUI. .patch
actually lists the commit history, one by one. Running am invokes the same
process as rebase, as far as I can tell.


-Brian

On Sun, Nov 23, 2014 at 2:44 PM, Markus Weimer <ma...@weimo.de> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Yes, that is what I tried to suggest. In cases where merges are hard,
> GitHub's diff can be used as a patch. As long as we figure out how to
> attribute the code to the right author. I'll try that with my next merge.
>
> Markus
>
>
> On 2014-11-22 14:03, Byung-Gon Chun wrote:
> > How about applying a patch file?
> >
> https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committers+Guide
> >
> >  On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de>
> > wrote:
> >
> > Hi,
> >
> > yes, this has happened to me before. [REEF-28] / #12 was quite
> > tricky to merge, in that case because it started out not as a clean
> > single commit over current master. I did not have the patience to
> > resolve all the merge conflicts, so I instead did the following:
> >
> > * Create a new branch myself. * Checkout all the changed files from
> > the PR * Commit & merge that branch.
> >
> > That commit can be found here:
> >
> >
> >
> https://github.com/apache/incubator-reef/commit/141e4a321061403fc777087207869695aedd3b84
> >
> >  As you can see, we lost the authorship attribution along the way,
> > as the code now looks like a contribution by me, while it was
> > written by Julia. I believe that could have been fixed with some
> > git magic to edit the author information.
> >
> > This approach doesn't always work, as it looses potential changes
> > to the *same* files committed. That wasn't the case here, luckily.
> >
> > I wonder whether a similar thing could be achieved by using the
> > patch provided by GitHub. Basically, one would
> >
> > * Create a new branch off of master * Merge that patch in * Commit,
> > merge and push
> >
> > That *should* work cleanly. All we need to do is find a way to fix
> > the authorship attribution.
> >
> > Thoughts?
> >
> > Markus
> >
> >
> > On 2014-11-22 07:06, Brian Cho wrote:
> >>>> Hi,
> >>>>
> >>>> I just finished merging REEF-29 / PR #15
> >>>>
> >>>> The PR resulted in some discussion and many changes (it
> >>>> wasn't a trivial PR). The changes happened as pushes to the
> >>>> PR branch, and simultaneously commits were made to master.
> >>>>
> >>>> Now, when I tried the `git rebase -i apache/master` method
> >>>> by picking the first commit, and squashing the rest, the
> >>>> rebase did not go cleanly at all. There were merge conflicts
> >>>> that I had to address, and even some merges that happened
> >>>> that were plain wrong. I ended up painstakingly addressing
> >>>> all the conflicts to manually match
> >>>> https://github.com/apache/incubator-reef/pull/15.diff
> >>>>
> >>>> Has anyone else had this experience? I feel something is
> >>>> wrong with our method, given that github is able to find the
> >>>> correct diff (the link above) without conflicts, but our
> >>>> method does the wrong thing. Please add to this thread if you
> >>>> have any similar adventures (now or in the future).
> >>>>
> >>>> Thanks, Brian
> >>>>
> >>
> >
> >
> >
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJUcXRIAAoJEOEJzzjJZ6pcWMkQAIXQ75DXo8ScwQg7Xcy+wT+k
> B9JIUPhg04jSr3f5jPFmlHrwSaSl4Z3ve5DzL6+ef5o7wbkMP4blUbXQO2wTgZlF
> FRF8hK8Iqn0TPRTTpkatp/5E9i56nGHr8aW1vT6ZFEPa8slalHZPIViGtOcrxNTn
> zSF432nqp32C9ENYDYKguzDjRse36PbzczQL2o0wrcmkqdriaHK3607JiPeKcXvu
> nCf6PkKrOEwRnQJ3VOFTJaNGv5zlCJwZ76VL9d/o0b3dndq9eIph+G7SKGuKJZEL
> YLlM0ZxPNFWPJlqlf8IzD+zZHEtqe5m0unIEyUGfTwEb6Ap3yr6ul3+Ly3TzKFBH
> X2xRbuhztQKCiSh56tg1NazrWApdA6gQFwIxHVlZFNHIb9EwXvOX4apMQBTdmhFl
> ipMPQjdkOJZHAC5UbvX7ZgKqn7XjUmRcEMoWOF1q6FxMkUZlZ/1+rdl3HfteGoJE
> JUqzDnXEDZmseHV5qmt+yK/Bf258CfKl6WotbANFNOROv24DY7eiwCYYgIJRJh9k
> 2eqqfs0m4qvERPeFa9WU8xt3mPey+DAL2SaIH476nKWJFyU09YgyvsR1Png5irgZ
> x/FnM7fj5mxxKhYMCyKpre4q4mTC6WwBpi2OHpW/VthOybmznGfpB8yozWHM+4Bl
> TrNDK+3rFRah0KjPJGXK
> =tADx
> -----END PGP SIGNATURE-----
>

Re: Adventures while merging

Posted by Markus Weimer <ma...@weimo.de>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yes, that is what I tried to suggest. In cases where merges are hard,
GitHub's diff can be used as a patch. As long as we figure out how to
attribute the code to the right author. I'll try that with my next merge.

Markus


On 2014-11-22 14:03, Byung-Gon Chun wrote:
> How about applying a patch file? 
> https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committers+Guide
>
>  On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de>
> wrote:
> 
> Hi,
> 
> yes, this has happened to me before. [REEF-28] / #12 was quite
> tricky to merge, in that case because it started out not as a clean
> single commit over current master. I did not have the patience to
> resolve all the merge conflicts, so I instead did the following:
> 
> * Create a new branch myself. * Checkout all the changed files from
> the PR * Commit & merge that branch.
> 
> That commit can be found here:
> 
> 
> https://github.com/apache/incubator-reef/commit/141e4a321061403fc777087207869695aedd3b84
>
>  As you can see, we lost the authorship attribution along the way,
> as the code now looks like a contribution by me, while it was
> written by Julia. I believe that could have been fixed with some
> git magic to edit the author information.
> 
> This approach doesn't always work, as it looses potential changes
> to the *same* files committed. That wasn't the case here, luckily.
> 
> I wonder whether a similar thing could be achieved by using the
> patch provided by GitHub. Basically, one would
> 
> * Create a new branch off of master * Merge that patch in * Commit,
> merge and push
> 
> That *should* work cleanly. All we need to do is find a way to fix
> the authorship attribution.
> 
> Thoughts?
> 
> Markus
> 
> 
> On 2014-11-22 07:06, Brian Cho wrote:
>>>> Hi,
>>>> 
>>>> I just finished merging REEF-29 / PR #15
>>>> 
>>>> The PR resulted in some discussion and many changes (it
>>>> wasn't a trivial PR). The changes happened as pushes to the
>>>> PR branch, and simultaneously commits were made to master.
>>>> 
>>>> Now, when I tried the `git rebase -i apache/master` method
>>>> by picking the first commit, and squashing the rest, the
>>>> rebase did not go cleanly at all. There were merge conflicts
>>>> that I had to address, and even some merges that happened
>>>> that were plain wrong. I ended up painstakingly addressing
>>>> all the conflicts to manually match
>>>> https://github.com/apache/incubator-reef/pull/15.diff
>>>> 
>>>> Has anyone else had this experience? I feel something is
>>>> wrong with our method, given that github is able to find the
>>>> correct diff (the link above) without conflicts, but our
>>>> method does the wrong thing. Please add to this thread if you
>>>> have any similar adventures (now or in the future).
>>>> 
>>>> Thanks, Brian
>>>> 
>> 
> 
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUcXRIAAoJEOEJzzjJZ6pcWMkQAIXQ75DXo8ScwQg7Xcy+wT+k
B9JIUPhg04jSr3f5jPFmlHrwSaSl4Z3ve5DzL6+ef5o7wbkMP4blUbXQO2wTgZlF
FRF8hK8Iqn0TPRTTpkatp/5E9i56nGHr8aW1vT6ZFEPa8slalHZPIViGtOcrxNTn
zSF432nqp32C9ENYDYKguzDjRse36PbzczQL2o0wrcmkqdriaHK3607JiPeKcXvu
nCf6PkKrOEwRnQJ3VOFTJaNGv5zlCJwZ76VL9d/o0b3dndq9eIph+G7SKGuKJZEL
YLlM0ZxPNFWPJlqlf8IzD+zZHEtqe5m0unIEyUGfTwEb6Ap3yr6ul3+Ly3TzKFBH
X2xRbuhztQKCiSh56tg1NazrWApdA6gQFwIxHVlZFNHIb9EwXvOX4apMQBTdmhFl
ipMPQjdkOJZHAC5UbvX7ZgKqn7XjUmRcEMoWOF1q6FxMkUZlZ/1+rdl3HfteGoJE
JUqzDnXEDZmseHV5qmt+yK/Bf258CfKl6WotbANFNOROv24DY7eiwCYYgIJRJh9k
2eqqfs0m4qvERPeFa9WU8xt3mPey+DAL2SaIH476nKWJFyU09YgyvsR1Png5irgZ
x/FnM7fj5mxxKhYMCyKpre4q4mTC6WwBpi2OHpW/VthOybmznGfpB8yozWHM+4Bl
TrNDK+3rFRah0KjPJGXK
=tADx
-----END PGP SIGNATURE-----

Re: Adventures while merging

Posted by Byung-Gon Chun <bg...@gmail.com>.
How about applying a patch file?
https://cwiki.apache.org/confluence/display/usergrid/Usergrid+Committers+Guide

On Sun, Nov 23, 2014 at 1:43 AM, Markus Weimer <ma...@weimo.de> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> yes, this has happened to me before. [REEF-28] / #12 was quite tricky
> to merge, in that case because it started out not as a clean single
> commit over current master. I did not have the patience to resolve all
> the merge conflicts, so I instead did the following:
>
>   * Create a new branch myself.
>   * Checkout all the changed files from the PR
>   * Commit & merge that branch.
>
> That commit can be found here:
>
>
> https://github.com/apache/incubator-reef/commit/141e4a321061403fc777087207869695aedd3b84
>
> As you can see, we lost the authorship attribution along the way, as
> the code now looks like a contribution by me, while it was written by
> Julia. I believe that could have been fixed with some git magic to
> edit the author information.
>
> This approach doesn't always work, as it looses potential changes to
> the *same* files committed. That wasn't the case here, luckily.
>
> I wonder whether a similar thing could be achieved by using the patch
> provided by GitHub. Basically, one would
>
>   * Create a new branch off of master
>   * Merge that patch in
>   * Commit, merge and push
>
> That *should* work cleanly. All we need to do is find a way to fix the
> authorship attribution.
>
> Thoughts?
>
> Markus
>
>
> On 2014-11-22 07:06, Brian Cho wrote:
> > Hi,
> >
> > I just finished merging REEF-29 / PR #15
> >
> > The PR resulted in some discussion and many changes (it wasn't a
> > trivial PR). The changes happened as pushes to the PR branch, and
> > simultaneously commits were made to master.
> >
> > Now, when I tried the `git rebase -i apache/master` method by
> > picking the first commit, and squashing the rest, the rebase did
> > not go cleanly at all. There were merge conflicts that I had to
> > address, and even some merges that happened that were plain wrong.
> > I ended up painstakingly addressing all the conflicts to manually
> > match https://github.com/apache/incubator-reef/pull/15.diff
> >
> > Has anyone else had this experience? I feel something is wrong with
> > our method, given that github is able to find the correct diff (the
> > link above) without conflicts, but our method does the wrong thing.
> > Please add to this thread if you have any similar adventures (now
> > or in the future).
> >
> > Thanks, Brian
> >
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJUcL01AAoJEOEJzzjJZ6pcrs4P/15u/mE4tmhj/R7tP4qnxlJV
> rVll/QGgknvDay6dL6KAgRuT2O9JUDJlZNviZJq0qHbBDuz0Z3Q0yk+siMx7gSXE
> ujWSnnCnN4cyEOHBcWCcIRVHok3P9mHPmhYuakGKhaHpkFQF0HdLzxz/mt01O6Po
> kslv2jvcyjyYAZ1LxgreMTD3H7yO0KpB/aV1a6RWheB7kybhZorldoo2JSYGSFBB
> 3GISvk/fFGLc7aWdUbDB4G5luovJgzY0kAd89qytUVrLc3Ii/uipVqiTt+EBqWXV
> khFuiD1mpMp952XwmTBAuJZV0Jn+AY5vrAou/uX1kaP6yyg1qsLcIL9txLtGrzJv
> KhgB8Zw3ZEIe5kaWggYNsrT87gsfSF9NG9/XK3soXFP92MKCTa09u+KIv5qtvmse
> sCKXnc8ERczhSupCJJ9ic2KAfw4D2tISmna60j6WCRGhsWdliZmnsOZuc3eJQsvr
> cHLoqz/i/+PHSyXk8iXkpN/jgSm9S38zlCJBXyySWzxMEJa+yKOASu1z3ofk4Cgm
> w1Bc/Zm0FcOylQv9KTXeTgBgWIwS+/CGXhGKzVmBVJ7TFoS38ht9zctHs26zeIbh
> EY9yPZtanSxPKfC/A0akgP6a3DTNFVQWoi4UmLPYI2j4fT1Z5HXkQBO5bi+QiXTe
> cyQ05Y39jzCG2lqyxij5
> =nPbm
> -----END PGP SIGNATURE-----
>



-- 
Byung-Gon Chun

Re: Adventures while merging

Posted by Markus Weimer <ma...@weimo.de>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

yes, this has happened to me before. [REEF-28] / #12 was quite tricky
to merge, in that case because it started out not as a clean single
commit over current master. I did not have the patience to resolve all
the merge conflicts, so I instead did the following:

  * Create a new branch myself.
  * Checkout all the changed files from the PR
  * Commit & merge that branch.

That commit can be found here:

https://github.com/apache/incubator-reef/commit/141e4a321061403fc777087207869695aedd3b84

As you can see, we lost the authorship attribution along the way, as
the code now looks like a contribution by me, while it was written by
Julia. I believe that could have been fixed with some git magic to
edit the author information.

This approach doesn't always work, as it looses potential changes to
the *same* files committed. That wasn't the case here, luckily.

I wonder whether a similar thing could be achieved by using the patch
provided by GitHub. Basically, one would

  * Create a new branch off of master
  * Merge that patch in
  * Commit, merge and push

That *should* work cleanly. All we need to do is find a way to fix the
authorship attribution.

Thoughts?

Markus


On 2014-11-22 07:06, Brian Cho wrote:
> Hi,
> 
> I just finished merging REEF-29 / PR #15
> 
> The PR resulted in some discussion and many changes (it wasn't a
> trivial PR). The changes happened as pushes to the PR branch, and
> simultaneously commits were made to master.
> 
> Now, when I tried the `git rebase -i apache/master` method by
> picking the first commit, and squashing the rest, the rebase did
> not go cleanly at all. There were merge conflicts that I had to
> address, and even some merges that happened that were plain wrong.
> I ended up painstakingly addressing all the conflicts to manually
> match https://github.com/apache/incubator-reef/pull/15.diff
> 
> Has anyone else had this experience? I feel something is wrong with
> our method, given that github is able to find the correct diff (the
> link above) without conflicts, but our method does the wrong thing.
> Please add to this thread if you have any similar adventures (now
> or in the future).
> 
> Thanks, Brian
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUcL01AAoJEOEJzzjJZ6pcrs4P/15u/mE4tmhj/R7tP4qnxlJV
rVll/QGgknvDay6dL6KAgRuT2O9JUDJlZNviZJq0qHbBDuz0Z3Q0yk+siMx7gSXE
ujWSnnCnN4cyEOHBcWCcIRVHok3P9mHPmhYuakGKhaHpkFQF0HdLzxz/mt01O6Po
kslv2jvcyjyYAZ1LxgreMTD3H7yO0KpB/aV1a6RWheB7kybhZorldoo2JSYGSFBB
3GISvk/fFGLc7aWdUbDB4G5luovJgzY0kAd89qytUVrLc3Ii/uipVqiTt+EBqWXV
khFuiD1mpMp952XwmTBAuJZV0Jn+AY5vrAou/uX1kaP6yyg1qsLcIL9txLtGrzJv
KhgB8Zw3ZEIe5kaWggYNsrT87gsfSF9NG9/XK3soXFP92MKCTa09u+KIv5qtvmse
sCKXnc8ERczhSupCJJ9ic2KAfw4D2tISmna60j6WCRGhsWdliZmnsOZuc3eJQsvr
cHLoqz/i/+PHSyXk8iXkpN/jgSm9S38zlCJBXyySWzxMEJa+yKOASu1z3ofk4Cgm
w1Bc/Zm0FcOylQv9KTXeTgBgWIwS+/CGXhGKzVmBVJ7TFoS38ht9zctHs26zeIbh
EY9yPZtanSxPKfC/A0akgP6a3DTNFVQWoi4UmLPYI2j4fT1Z5HXkQBO5bi+QiXTe
cyQ05Y39jzCG2lqyxij5
=nPbm
-----END PGP SIGNATURE-----