Resolving pull request conflicts

[Mojca Miklavec]

On 23 February 2017 at 12:04, Ryan Schmidt wrote:
>
> 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.

This sounds like a totally reasonable way to do stuff.

With lots of things that are still not clear to me (for one, I still
don't see where all your commits are; I see a link posted by Zero
King, but don't know how to actually reach that content from
elewhere). Initially it looked like a merge commit somewhere in
master. Then I found it in a branch that doesn't even seem to exist in
macports/macports-ports.

> 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?

It's certainly better to have something that doesn't require
additional manual work. (It would be even better if GitHub offered a
way to perform certain operations online that it currently doesn't.)

I'm currently not sure what exactly is involved in avoiding a merge
before pushing things to master. If clicking on a green button on the
web interface would get the job done without showing the merge commit,
that would be great. (Maybe we need to test an an auxiliary repo.)

>> 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.

The only strange thing here is
    https://github.com/macports/macports-ports/commit/2f8d7fd90a9f0a66b6aa438c78128eec15c46d79
that was linked by Zero King. But I admit that it's not clear to me
where that commit comes from (how to get to it from anywhere else).

Did l2dy remove it from the Pull Request already and overwrite it with
another one?

>> 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.

Yes, that's the idea. Keeping committing to the pull request until
it's ready and clean it before it goes to master.

I would say that in many cases with have PRs that are ready to be
committed (from the perspective of changes that they introduced), but
not ready from the perspective of "clean commit messages" and having
way too many commits with trivial bugfixes that should be cleaned up
eventually. And in those cases there's still manual work needed to
squash and reword commits and we often ask authors to do that, so that
someone can eventually "click the green button".

Again, I still don't quite understand where that merge commit comes
from, but Zero King's complaint was probably that after your change
the PR might have been in a similar "intermediate" state where there
would be still manual work needed to get it directly to master.

> (The "squash-and-merge" GitHub button that we haven't enabled yet.)

If that works as expected, this could indeed solve the problem by
making it a matter of a button click to fix the commits (provided one
doesn't click the wrong button of course :).

Mojca