JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.64k stars 5.48k forks source link

Require squash-merging for master branch? #52985

Open stevengj opened 9 months ago

stevengj commented 9 months ago

Is it possible to set the master branch to disable merging without squashing by default?

See instructions at: https://github.com/orgs/community/discussions/50593#discussioncomment-5402508 Unfortunately, the linked instructions appear to be wrong. But is there another way to do it? At the very least, it seems we could require linear history.

Merging without squashing is something that should only be done in exceptional circumstances — not only does it clutter the commit history, but it also potentially merges commits that are in a broken state, breaking git bisect. But right now, it is way too easy to do by accident (e.g. see #52777).

As was commented in #52777, at the very least this should be in the developer docs somewhere. This policy is documented in the "Git Recommendations For Pull Request Reviewers" section of the CONTRIBUTING.md guide, which should be updated if there is a change in repository settings.

DilumAluthge commented 9 months ago

In the repo settings, we have the ability to disable regular (non-squash) merging. This would then force everyone to always use squash merging.

As far as I can tell, there's no way to set the default to be squash-merging but still allow users to regular merge if they really want.

stevengj commented 9 months ago

Where is that setting?

Non-squash merging seems to be so rare that I think it would be fine to require an admin to go in and change that setting, then do the regular merge, then change the setting back.

DilumAluthge commented 9 months ago

It's in the repo settings. Control-F the general repo settings page for the checkbox labeled "Allow merge commits", and uncheck that checkbox.

If we want to try it out as an experiment, I can uncheck that checkbox.

stevengj commented 9 months ago

Great, I didn't see that! We should also uncheck "Allow rebase merging", so that squash merging is the only option.

DilumAluthge commented 9 months ago

We should also uncheck "Allow rebase merging"

This is already done.

stevengj commented 9 months ago

The "Git Recommendations For Pull Request Reviewers" section of the CONTRIBUTING.md guide should also be updated to say that squash-merge is required, but non-squashed merges can be performed by admins in exceptional cases.

nsajko commented 9 months ago

These are the PRs with label don't squash: https://github.com/JuliaLang/julia/issues?q=label%3A%22don%27t+squash%22+

IanButterworth commented 9 months ago

Regarding rebase merge, I have a request open for GitHub to add the option to link back to the PR in the rebase merge commits, like it does for squash merges, which is currently a reason why rebase merge shouldn't be used at all for this project https://github.com/orgs/community/discussions/62118

With that change it would become the preferred non-squash approach, I think

vchuravy commented 9 months ago

Yeah GitHub stack PR support is not great so sometimes not squashing is the preferred course of action.