Consider using Squash & Merge, at least sometimes

[minrk]

Proposed change

Consider using Squash & Merge to avoid conversations about rebase, etc.

In general, I prefer merge commits for PRs. But what I don't like are conversations about 'cleaning up the history', especially as a hurdle for new contributors. Like autoformatting, while I like the result of squash & merge less than merge most of the time, I very much like that a whole category of not-especially-useful conversation can be eliminated and the trade off is often worth it.

Alternative options

Current policy: ~always merge commit. Rebase / squash PRs that get out of hand before merging. Squash & merge available, but not discussed as an official option.

Advantages of always merge:

• git log --merges always lists all pull requests
• 'large' PRs of multi-step changes are reflected well
• Multi-contributor PRs are accurately reflected
• git bisect, etc. isolate smaller changes
• Allows accurate/deliberate git history

Disadvantages of always merge:

• need to rebase to exclude commits from history (whether for technical or aesthetic reasons)

Advantages of always squash:

• Never need to discuss 'clean' history, PRs never need to rebase, etc.
• Single commit per PR (git bisect, etc. always resolve to 'complete' changes rather than partial in-progress commits)
• Simple linear git history (I don't consider this is a benefit, but others do)

Not-quite-disadvantages of squash:

• If desired, contributor history is preserved, but only in the PR itself, not the main branch (i.e. it's still accessible in practice, but lives specifically on GitHub, not in git)

Advantages of squash as an option, but merge as default (or vice versa):

• All the benefits of always merge, but
• Squash still allows eliminating conversations about rebasing, etc. when a PR gets overly complex commit-wise compared to changes (e.g. new contributor with several "fix X", "Update some-doc.md" commits) at reviewers discretion. Essentially, whenever you feel like asking for a rebase, merge with squash instead, otherwise merge is fine.

Disadvantages of multiple options:

• "Reviewer's discretion" above adds a decision to make (this decision already exists, but now it's about rebase instead of squash)
• inconsistent history (neither merge-per-pr, nor entirely linear history)

Who would use this feature?

JupyterHub maintainers, PR contributors to a lesser extent

[manics]

I like the idea of defaulting to merge, with squashing as an option. It leaves the discretion to the person merging, but without changing our existing process too much. If you're unsure, don't want to make a decision, or just forget, then merge as at present. Otherwise you can choose to squash.

[minrk]

Yeah, I've been thinking about writing some more things like this down based on recent discussions around unclear expectations. I feel like we already have this, but without it being written down, everyone is left to their own wonderings about "is this okay?" even when the answer is always "yes." I want to say "yes, it's fine, I won't tell folks what to do, but here's a way to think about it, if it helps."

[sgibson91]

I love this comment @minrk and think it is a good approach. Tbh, I think a great place to start would be to just write down any/all processes we follow ad hoc now and get them on the team compass. And then we can iterate with issues/PRs to improve/adapt/clarify as needed and on whatever timescale suits us.

[yuvipanda]

Sometimes if I'm not feeling great, I'll go look at a graph like this somewhere:

and feel better. Defaulting to squash of course makes the bars smaller. I know I'm not the only person for whom this is helpful

So the primary problem with squash to me is that if it's a reviewer, often a core contributor of the project, asks a contributor (often new), if it's ok to squash, they'll most likely say yes. It is definitely often easier than trying to rebase for cleanliness. But at least early on, I think these kinda vanity metrics are important.

So overall I'm not opposed to 'merge always, but squash sometimes'. But I'd like us to be very explicit about the conditions under which we'd ask for something to be squashed, so we don't end up in a situation where it almost purely ends up depending on who is the reviewer.

For me personally, I'm totally ok just not caring about rebasing for aesthetic reasons! If I'm git bisecting and end up with a commit that's not very descriptive, I can put the commit hash in github and it'll immediately give me the PR that that commit was from. This happens so rarely that the tradeoff is totally worth it for me.