Squash merge?

[jrbourbeau]

I don't feel super strongly, but have a mild preference for using squash merging on most projects. This makes it so contributors don't have to be careful with individual commits (e.g. git commit -m "typo", git commit -m "Ah, forgot to lint") in their PRs and the git log on main is relatively clean (one commit per PR).

Thoughts from others on moving to sqash merging?

[MattF-NSIDC]

I prefer to use squash merging on a case-by-case basis. In PRs with well-curated commits, I like to keep them as-is. When we get PRs with poorly-curated commits, I'm all for a squash merge!

[jhkennedy]

I personally don't think squash merges solve anything but vanity^ -- but I also don't really care what's the norm here, so squashes are fine by me if that's the preference.

I only have 2 actual concerns, and that's true with any merge method:

1. I want the green merge button by default to just do what the norm is so maintainers don't really have to think about it (helps expand the maintainer team!)
2. CI/CD often comes with assumptions around how things are merged, so we just need to make sure it's consistent with the allowed merge methods

so it might be better to just pick one of these, and dis-allow the rest:

---

^ It can be argued that squash merges make git bisect easier, but you lose the journey/context that's often helpful for debugging without GitHub, so offline debugging is harder. Overall the positives and the negatives end up in the "meh" category for me and it ends up just being a discussion of what's prettier.

[MattF-NSIDC]

As I'm thinking about this I keep changing my mind. I don't think there's any "correct" decision here...

I think "is a clean history important?" is the root question, and I think "yes", but not above all, especially not above community health or inclusivity. I like a clean history, and I like to curate my commits to the best of my ability (after a particularly bad day, that ability can leave my body 😆), but I don't think we should leverage that expectation on contributors.

I think the best solution may just to be loosey-goosey about it, make it a "best effort" kind of thing and it'll work out fine. Encourage merge commits for well-curated PRs and squashes for messier ones, but be OK with people selecting the default and sometimes having messy PRs not get squashed. You can still get a clean one-commit-per-PR log view of main with merge commits.

The strongest opinion I have on this, I think, is that we should disable rebase merging, because even with well-curated commits, I'd rather only see the merge commit on main than all of them. Rebasing is also not nearly as broadly understood as merging, and I remember being intimidated by rebasing myself at one time :)

[jhkennedy]

I don't think there's any "correct" decision here...

💯

The strongest opinion I have on this, I think, is that we should disable rebase merging, because even with well-curated commits, I'd rather only see the merge commit on main than all of them. Rebasing is also not nearly as broadly understood as merging, and I remember being intimidated by rebasing myself at one time :)

I can agree to that and am happy to uncheck that one at least 😄

[mfisher87]

@yuvipanda makes an excellent point here: jupyterhub/team-compass#686 (comment)

And thinking back on how I merged #566, I apologize @botanical for not considering how the choice might affect you. Personally, Yuvi's argument has swayed me and now I have stronger feelings :)

[mfisher87]

I feel like a good outcome from this ticket may be either:

• Pick one strategy and disallow others (simple, not nuanced)
• Disable "rebase" strategy and write a documentation page/section on our merge-vs-squash policy and our motivations behind it -- e.g. "merge by default, sometimes squash - be wary of power dynamics and let the contributor's preference take precedence over the maintainer's"? (not simple, nuanced)

I feel like the latter may be the right way for us, and I know a lot of people are pro-squash... happy to defer if nobody else is swayed, but I do think inclusiveness is more important than cleanliness here.

[chuckwondo]

My OCD tendencies generally lead me to keeping my git histories "cleaner" (for better or worse), but I am happy to agree on the "merge by default, sometimes squash" approach here.

[JessicaS11]

Personally, I prefer squashing because it means I'm more likely to commit more often (e.g. at the end of the workday I can say "WIP blah blah" because my half-baked idea won't be part of the main/dev branch history). This means I worry less about "airing my dirty laundry" while I'm figuring things out. As a maintainer who's not a git wizard, I also find it far easier to see how the library has progressed if I can read the history without every single "fix typo" commit showing up. It also helps if others aren't writing good commit messages...

All that said, I support @MattF-NSIDC's second bullet:

Disable "rebase" strategy and write a documentation page/section on our merge-vs-squash policy and our motivations behind it -- e.g. "merge by default, sometimes squash - be wary of power dynamics and let the contributor's preference take precedence over the maintainer's"? (not simple, nuanced)

Is there a way to have a required checkbox when a PR is created that allows the contributor to easily specify which type of merge they'd prefer? I'm thinking about how we don't end up with approved PRs hanging until the contributor comes back and merges.

[jhkennedy]

Personally, I prefer squashing because it means I'm more likely to commit more often (e.g. at the end of the workday I can say "WIP blah blah" because my half-baked idea won't be part of the main/dev branch history).

@JessicaS11 I would not hesitate to commit whether we're doing squash merges or merge commits; development is messy, history is messy, and it's fine.

As a maintainer who's not a git wizard, I also find it far easier to see how the library has progressed if I can read the history without every single "fix typo" commit showing up. It also helps if others aren't writing good commit messages...

That's a good point; there are a few commands and aliases you can use to filter for this with git to reduce the clutter you have to filter through.

I don't have them on hand since I don't mind the mess when I look at git history (my team at ASF uses merge commits, and we don't bother rebasing/squashing out those things), but I'll circle back with them, which we could document.

[jhkennedy]

Is there a way to have a required checkbox when a PR is created that allows the contributor to easily specify which type of merge they'd prefer?

We could add it to the template, or automate a comment on PRs asking, but I don't think there's a robust way to require it. I also think, generally, that puts too much on contributors as "Would you prefer a merge commit or a squash merge?" requires quite a bit of git knowledge to make an informed decision.

[mfisher87]

Is there a way to have a required checkbox when a PR is created that allows the contributor to easily specify which type of merge they'd prefer?

We could add it to the template, or automate a comment on PRs asking, but I don't think there's a robust way to require it. I also think, generally, that puts too much on contributors as "Would you prefer a merge commit or a squash merge?" requires quite a bit of git knowledge to make an informed decision.

I really like these ideas. I think we can make it easy on contributors by treating it like a consent request in the PR template. By default, if they don't take any action, we don't squash. If we feel the need to do changes to the history in a PR, we can discuss it with the contributor as usual!

• OPTIONAL Check this box by changing [ ] to [x] at the start of the line if you are OK with having your commits "squashed" when this change is accepted. This helps us keep a clean history by combining many commits in to one, but some contributors may prefer to keep their commits as-is (for example, one commit adds a failing test to reproduce a bug, another commit fixes the bug), and that's OK.

Thoughts?