doc: Switch release-notes.md to union merge #12823

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1803-docGitattributes changing 1 files +1 −0

MarcoFalke commented at 7:26 PM on March 28, 2018: member

Fix #12819
MarcoFalke added the label Docs on Mar 28, 2018
jnewbery commented at 7:55 PM on March 28, 2018: member

Concept ACK. Why not just place the new rule in the base directory .gitattributes file?
doc: Switch release-notes.md to union merge 5be024a0ed
MarcoFalke force-pushed on Mar 28, 2018
MarcoFalke renamed this:
~~doc: Add .gitattributes file for release-notes.md~~
doc: Switch release-notes.md to union merge
on Mar 28, 2018
MarcoFalke commented at 8:08 PM on March 28, 2018: member

Thanks, I missed that there was one
instagibbs commented at 8:11 PM on March 28, 2018: member

huh TIL.

concept ACK
jnewbery commented at 8:13 PM on March 28, 2018: member

Tested ACK 5be024a0ed891fff202106cfc046d6e90f265a99
meshcollider commented at 8:40 PM on March 28, 2018: contributor

Nice, utACK https://github.com/bitcoin/bitcoin/pull/12823/commits/5be024a0ed891fff202106cfc046d6e90f265a99
randolf approved
ajtowns commented at 12:55 AM on March 29, 2018: member

This works fine locally but github will still report a merge conflict. See https://github.com/isaacs/github/issues/487 for background and https://github.com/ajtowns/test-repo/pull/8 for an example.
MarcoFalke commented at 12:35 PM on March 29, 2018: member

We don't use GitHub for merges, so I guess that is fine.
laanwj commented at 1:33 PM on March 29, 2018: member

Can you elaborate on what the result of this will be? The git documentation says this on union merges:

Instead of leaving conflicts in the file, resolve conflicts favouring our (or their or both) side of the lines.

Does this mean it prefers losing information (by taking from either side) to adding conflicts?
MarcoFalke commented at 1:54 PM on March 29, 2018: member

The documentation is for all three options, which explains the weird structure of the sentence.

The explanation for --union is "Instead of leaving conflicts in the file, resolve conflicts favouring <strike>our (or their or</strike> both) side of the lines."

https://git-scm.com/docs/git-merge-file#git-merge-file---union
MarcoFalke commented at 1:56 PM on March 29, 2018: member

Which means that for any conflict it will include both sides. That is fine if both sides add stuff, but does not work too well if both sides remove the same line. That would mean someone had to solve the "union merge conflict" after merge.
jnewbery commented at 2:11 PM on March 29, 2018: member

We don't use GitHub for merges, so I guess that is fine.

I think the benefit of this PR would be if it didn't cause GitHub to say that a PR has conflicts (and therefore needs rebase). If github still thinks there's a conflict then the PRs will still require a rebase (and therefore invalidate ACKs).
laanwj commented at 2:18 PM on March 29, 2018: member

Thanks for the explanation.

Any idea how this will interact with #12708? Will git still see the union-result as a "clean merge"?
MarcoFalke commented at 2:45 PM on March 29, 2018: member

It should, since it uses git merge internally and that should respect the attributes file.
practicalswift commented at 3:03 PM on March 29, 2018: contributor

Concept ACK

Good idea
MarcoFalke closed this on Mar 29, 2018
MarcoFalke deleted the branch on Mar 29, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12819 Avoid release-notes.md conflicts