Resolving pull request conflicts

Thu Feb 23 11:04:38 UTC 2017

On Feb 23, 2017, at 04:46, Mojca Miklavec <mojca at macports.org> wrote:
> 
> Dear Ryan,
> 
> On 23 February 2017 at 11:19, Ryan Schmidt  wrote:
>> Browsing pull requests, I found this one:
>> 
>> https://github.com/macports/macports-ports/pull/305
>> 
>> Another commit had happened to that port in the mean time, so the PR now had conflicts and could not be applied as-is. GitHub invited me to edit the file on the web to resolve the conflict, so I did. I was then asked by the PR author not to do that. Can someone explain why resolving the conflict was bad?
> 
> It's not resolving the conflict that's bad.
> 
> I guess it's just that the GitHub's user interface is anything but
> optimal when it comes to certain aspects.
> 
> I'm still confused about what exactly you did to end up in that
> situation (but it's probably more clear to me now that you mentioned
> it; perhaps I should this on some test repository).

GitHub said the pull request had a conflict. (The PR is a version update, but a revbump was committed to the port in the meantime.) GitHub said I could resolve the conflict on the command line or in a web-based editor. There was a green button inviting me to start resolving the conflict on the web. I clicked this and was brought to a web-based text editor showing the Portfile in its conflicted state. I edited the file, removing the conflict markers and incorrect lines. I clicked the button saying I have now resolved the conflicts.

> I agree with Zero King that it makes more sense to keep the branches
> clean.

What does it mean to be "clean"?

Is it better to have a pull request that has conflicts which cannot be merged without additional manual work, or one that has had those conflicts resolved?

> If anything it's confusing to see 20 unrelated commits when
> it's about merging some trivial changes in an unrelated file.

I'm not sure what you mean here.

> We kind of decided that we'll try to keep our history linear & clean.

Absolutely the history that appears in master should be "clean", meaning one commit per logical change. But it is natural, is it not, that a pull request might need refinement before being merged, to address feedback or in this case conflicts, and that that refinement would come in the form of additional commits? My understanding was that once it was deemed satisfactory such multi-commit PRs would be squashed into a single commit before/during merging. (The "squash-and-merge" GitHub button that we haven't enabled yet.)

> (I'm sure you still remember Andrea's 300 duplicated commits in a
> failed attempt to do so and probably agree that it makes sense to do
> our best to avoid repeating such type of commits?)

I do remember that occurrence, and not understanding how it happened.

> Ever since I discovered that GitHub won't use my email when I click on
> the merge button in pull requests, I never use the GitHub's GUI to do
> any operations like this one since I never know what it will do behind
> the scene.

I guess I'll just continue not to participate in anything other than the most straightforward pull requests for now, since I still find the git command line too complicated to understand.

> Here's what I do when I work on Pull Requests.
> 
> 1.) First of all I made sure to add
> 
>    [pull]
>        rebase = true
> 
> to .git/config just in case that I would forget to call "--rebase"
> when I do "git pull". If that option is missing, one should always try
> to run "git pull origin master --rebase" on the local master branch to
> avoid ending up with weird merges.
> 
> 2.) Here's what I do to fetch the changes locally and push them back:
> 
> # set these three lines manually
> PR_N=123
> PR_USER=some-username
> PR_REMOTEBR=remote-branch-name
> 
> PR_LOCALBR=pr${PR_N}-${PR_USER}-${PR_REMOTEBR}
> 
> git fetch https://github.com/$PR_USER/macports-ports.git $PR_REMOTEBR
> git checkout -b $PR_LOCALBR FETCH_HEAD
> git rebase master
> 
> ## then do whatever "magic" is needed
> ## very often using commands like
> # git rebase -i HEAD~4
> # git commit --amend [--date="..."]
> 
> ## then check with
> # git log --graph
> ## to make sure that the history and commit messages are OK
> 
> # then update the branch in PR first
> # (this is also important to make the PR marked as "merged" rather
> than "closed")
> git push -f https://github.com/$PR_USER/macports-ports.git
> $PR_LOCALBR:$PR_REMOTEBR
> 
> # then check if the PR looks ok on the web just in case
> 
> # and finally push the changes to master
> git push origin $PR_LOCALBR:master
> 
> 
> Maybe some software already does most of that in a more user-friendly way.

Thanks for trying to help. This is just really complicated to me and not something I'm likely to do right now.

I find it remarkable that people find this easier than just downloading patchfiles from Trac...

> I'm not sure about the proper way to resolve conflicts, but git offers
> some command line interface. When I was committing the "$Id" stuff, it
> was less work to start from scratch that to keep resolving conflicts.
> And very often the commits are so simple that I simple go "from
> scratch".