Propose defaulting to Rebase & Merge

[jedahan]

What's the problem you want to solved? Merge commits create unnecessary noise and possible confusion in the git history.

Is there a solution you'd like to recommend? For #95 I tried out Rebase & Merge over just the simple Merge. This had a number of effects, all of which I think were positive:

1. Kept the commit history from the branch
2. Moved closed to the default C4 branchless dev ideology
3. Unexpectedly interesting: showed the maintainer name next to the committer name "with jedahan" which felt good by highlighting the collaboration and reinforcing the collaborator-maintainer separation.

This is related to #54 , but I think instead of having that languish on forever, I'd like to branch the discussion here, and make a PR with the change.

Proposed solution: default to rebase & merge, and capture that somewhere, likely contributing.md

[christianbundy]

Thanks for opening this discussion. I'd be comfortable experimenting with this, but my first reaction is hesitance and caution.

Merge commits create unnecessary noise and possible confusion in the git history.

I agree that a branch with merge commits is aesthetically bleh, but I see them like log files where the verbosity and usefulness are correlated. Keeping the real Git history means that we can see what the tree looked like during development, which makes git bisect and such way more useful. I think this page on rebase vs merge sums up the trade-offs, but the biggest red flags for rebases (for me) are:

• Rebase is not the default on GitHub or most Git tutorials I've seen, and I don't think it's super familiar to most Git users. I feel pretty comfortable with rebasing, but my intuition is that it gives most beginners :scream: vibes.
• Rebase changes history, which means it's hard to reason about whether a given commit has been merged into master. Maybe there are tricks for this? I've used Git for more than a decade but this isn't something that I'm familiar with.
• Rebase breaks signed commits, which I usually use (although I haven't re-enabled them since reinstalling my OS), although I don't need these to work.

Pivoting a bit, I think the reason that we're seeing lots of merge commits is that I've enabled one of GitHub's branch protection options:

Require branches to be up to date before merging
This ensures pull requests targeting a matching branch have been tested with the latest code. This setting will not take effect unless at least one status check is enabled (see below).

What happened in #95 is that I clicked 'merge master into this branch' on GitHub, immediately forgot that I'd done that, and then ran git pull origin master on the command-line. I wasn't very considerate about my merge conflicts and didn't think they'd affect anyone, so I just pushed up the entire hairball.

Two other options I'd like to highlight:

• In the future, I can be more considerate and avoid pushing merge-commit hairballs for review. :upside_down_face: I'll probably be more careful about not git pulling in two places at once, but if it does happen I can promise to rebase locally instead of asking you to review my commit spaghetti.
• We could also remove the 'require branches to be up-to-date before merging' option, which would vastly reduce the number of merge commits in these branches. I think this would be a good idea, especially if we're developing in C4-style private forks where that option won't be available to maintainers. The only downside is that in a hypothetical future where we have lots of tests, it's possible to have a branch that would be out-of-date with master that passes all tests and has no merge conflicts, but would cause test failures in master. I think the benefit is worth the risk, especially since we auto-run tests in master. OPTIMISTIC MERGING FTW.

Summary of my feelings:

• Rebase: caution, skepticism, :scream:, but okay with trying
• Not pushing messy branches: :heavy_check_mark: :angel:
• Disabling 'up-to-date before merging': :heavy_check_mark: :100:

[christianbundy]

Also, I've noticed that you've been developing on your personal fork -- cool! I'll join you. Should I remove my branches on this repo and add branch protection so we can't push anything accidentally?

[jedahan]

Thats a very thoughful reply. My gut feeling is disabling up-to-date before merging is mostly good. I feel like I haven't experienced all the possible development workflows to understand the drawbacks and benefits of each. In some ways, this makes me an ideal instigator because I've got a bit more of a beginners mind.

I think a subtle distinction to be made is the difference between encouraging git rebase vs Github Rebase & Merge button. From what I understand, Github Rebase & Merge is equivalent to "Disabling up-to-date before merging" except the web interface does all the work for you. At least in the case of #95 thats what it felt like to me.

The downsides of signed commits and "is commit X in main?" is real though.

edit: after reading the kernel document, I am now more confused the implications of requiring up-to-date before merging, and the multiple merge strategies.

Whatever decisions are to be made, as long as we think it makes contributing easier, especially for beginners, let's do that. I need to sleep on it or pair with someone to understand it all.

This also makes me wanna try Pijul :)

[christianbundy]

@jedahan How have the past few merges been? Better without all of the superfluous merging, or is this still a pain point for you?

[jedahan]

Its been good, happy to close.