Skip to content

Switch to squash commits #14952

Closed as not planned
Closed as not planned

Description

@nstarman

What is the problem this feature will solve?

We use merge commits, which preserve all the commits in a PR. While this sounds good, in practice the costs outweigh the benefits. In particular, it is a significant hurdle for contributors.

Pros:

  • breaking up a large PR into more atomic commits in the git history
  • Reverting a change

Cons:

  • Experienced contributors constantly squashing commits in their own PRs
  • Having to teach new contributors how to squash commits
  • Having to review a PR and think about the number of commits in the PR versus the actual content.

Practically though, how often do we revert? And how often do we comb the git history? The answer to both feels like "not too often". Since most of our PRs are small, trying to understand a change by looking at a single commit in the git history versus a whole PR will be minimally impacted. Only the rarer large PR will make git combing / bisecting harder.

OTOH we have this (in my experience) large barrier to entry by asking contributors to master the minutiae of git.
It starts with a maintainer making a seemingly innocuous request like "please squash to a few atomic commits", but if the current set of commits its messy, with half an idea in one commit and the other half in another, then this is a highly non-trivial request. Changing the git history can be very challenging unless one is doing the simplest of operations, like squashing adjacent commits. Rearranging commits, splitting a commit in half and combining it with other commits, etc. are all hard, yet we currently expect this in PRs. Why? Why don't we just squash the commits.

One PR => one "big" idea in the git history
If you want to see all the little ideas, don't look at the git history, look at the PR which records the discussion and all commits that got squashed.

Describe the desired outcome

We switch to Squash Commits.
So many projects, big and small, use this simple solution to lower maintainer burden and barrier-to-entry.

Contributors don't need to master git, they can just push as many commits as they want.
Reviewers don't need to think about the commits -- how atomic they are, how many there should be -- only the actual contribution to Astropy.
When a PR is ready we don't need to then wait to squash it down, it's just good to (squash) merge right away.

Additional context

I have heard from a number of people that they would contribute to Astropy but find it overly burdensome because of how our PR review process is set up. They want a detailed and automated CI and expect their PRs to be merged if they pass the CI, with minimal back and forth with reviewers about style and commits, only about the actual code performance, design, and relevancy.
IMO me hearing this is a large problem. If I'm hearing this from a few people, how many am I not hearing from?
This is one of the many reasons some of us originally championed black and Ruff: to eliminate style discussions in contributions.
While the ruff adoption is ongoing, due to our large technical debt, I think black has been a smashing success.
IMO squashing commits is the next way we lower the barrier to contribution and reduce the time from PR opening to merge.

### Tasks
- [ ] Enable squash commits
- [ ] Disable merge commits
- [ ] Live long and prosper

Activity

  1. eerovaher commented on Jun 16, 2023

    @eerovaher
    Member

    I'm closing this because it is a duplicate of #14473. Do post your ideas there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions