doc: Explain squashing with merge commits #25165
Conversation
|
🚨 Warning, this is untested, but tests and review are welcome. |
david-bakin
left a comment
david-bakin
left a comment
There was a problem hiding this comment.
What's wrong with a git merge --squash which I see is not recommended earlier in this document or in your update? (I really don't know, BTW, which is why I'm asking.)
jonatack
left a comment
jonatack
left a comment
There was a problem hiding this comment.
-0 on adding further git documentation to the repo, there are other and better resources
AFAICS you use pre-prepared maintainer responses and could add an appropriate link to an external resource where needed. |
|
Concept ACK. I think having some easy to follow steps here is useful. I do agree with @jonatack that going into too much details about git usage is out of scope. |
For me, this does not work: |
I agree. Mind sharing a link to the "other and better resources" that can be used? |
|
Recall that we are probably the only open source project that doesn't use the "squash on merge" GitHub feature, so I don't think there is documentation about this out there, but I am more than happy to be proven wrong. |
There was a problem hiding this comment.
I agree. Mind sharing a link to the "other and better resources" that can be used?
A prepared response may be a better place than the repo for an external link, but here are a couple that are similar to your method (rather than, say, the more flexible git rebase -i).
Yes, this is where I got the idea from. However, they rely on commit-counting, which is non-trivial with merge commits. |
maflcko
force-pushed
the
2205-doc-squash-
branch
2 times, most recently
from
fa81375 to
8888903
Compare
|
Changed to a more general approach (that may involve fixing conflicts again) |
That didn't work for my case in #25161 unfortunately. |
|
Can you elaborate a bit on this? I tried it on #25161 and it worked for me. |
|
I must have done something wrong then. I can't remember exactly what happened but I think I actually accidentally pulled and pushed after running |
Very likely I'm missing something fundamental here, but why would a change in a PR ever contain a merge commit? Is this for the scenario of "PR foo is based on PR bar, PR bar has been merged, now PR foo needs to rebase?" |
|
It usually happens when the author merges with current Bitcoin Core |
There was a problem hiding this comment.
ACK 888890302edacf3392957ef9e682cd6cb17a5f5c
Alternatively, could move the two git lines to the rebase section below (where currently concrete git instructions are missing, it only says that "git rebase" can be used, but without example) and link to there, e.g. "you will need to remove the merge commit first by rebasing on the target branch".
|
Thx, done. |
|
Aside, just came across https://github.com/lsilva01/operating-bitcoin-core-v1/blob/main/git-tutorial.md ... maybe reply to people with https://github.com/lsilva01/operating-bitcoin-core-v1/blob/main/git-tutorial.md#2-squashing-and-rebasing when useful. (I'm not sure how sustainable it is for a contributor to not become familiar with |
|
Yeah, I am happy to review a pull request that removes the sections in our readme and replaces them with a link. Or should I do it here? |
|
ACK fa2d226 |
fa2d226 doc: Explain squashing with merge commits (MacroFake) Pull request description: This avoids having to explain it in each thread ACKs for top commit: laanwj: ACK fa2d226 Tree-SHA512: e1533ee7c0ab0101c78aaebed97dc889b5eb941cf4c2dfbabbb5f0ec1bb7b1313a1a2e2405235d68c761f039373cebac67ce691a72c820a9252429d50c1ac7d5
8 participants
This avoids having to explain it in each thread