Automate Squash Merging to Reduce PR Friction and Improve CI Efficiency in homebrew/homebrew-core

[0xdevalias]

Verification

• This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

Provide a detailed description of the proposed feature

In the current workflow for handling pull requests (PRs) in the Homebrew repository, contributors are often required to manually squash their commits before a PR can be merged. This requirement can add unnecessary friction to the contribution process and may lead to additional complications, such as CI (Continuous Integration) blocking PRs due to poorly named commits within them.

What is the motivation for the feature?

The primary motivation for automating squash merging is to streamline the contribution process by reducing the manual steps required from contributors. Automating this process can also enhance the efficiency and reliability of CI checks, as it ensures that only the final, squashed commit is evaluated, avoiding issues with individual commit names.

---

can you squash all the commits?

@chenrui333 I've squashed the commits and force pushed.. but honestly that really shouldn't be a blocker in the automation on this repo. PR's can be squash merged, which would solve this and other 'friction points' (like the CI blocking a PR because a certain commit within it is named poorly) without requiring manual process on each and every PR.

Originally posted by @0xdevalias in Homebrew/homebrew-core#155716 (comment)

How will the feature be relevant to at least 90% of Homebrew users?

While this feature might seem to directly impact only contributors, its indirect benefits extend to the vast majority of Homebrew users. By simplifying the contribution process and improving CI reliability, the overall quality and timeliness of updates and new features in Homebrew can be enhanced. This leads to a more robust and reliable experience for all users who rely on Homebrew for software installation and management.

What alternatives to the feature have been considered?

1. Continuing with Manual Squashing: This is the current approach, but it adds extra steps for contributors and maintains potential CI issues.
2. Selective CI Checks: Adjusting CI to ignore certain commit-naming issues. However, this could lead to a decrease in overall commit quality.

Given these considerations, automating squash merging appears to be a more efficient and user-friendly approach to maintaining high-quality contributions and a smooth workflow in the Homebrew repository.

[0xdevalias]

While I haven't read the full thread, there seems to have been some past discussion at least tangentially related to this in the following issue:

• Discussion: ideal CI workflow homebrew-core#31511

[ZhongRuoyu]

First of all, thank you for your writeup and contributions.

Open source projects, especially large ones, usually have their own contribution standards that maintainers and contributors need to follow. In homebrew/core, we enforce the "one formula per commit; one commit per formula" rule, so modifications to a formula needs to be squashed into one commit in every PR. (Many of our contributors know this, but they sometimes forget about this when they push new changes, apply review suggestions, etc.) This ensures tidiness in the Git log and can be useful when one needs to browse the history of a file. The full commit style guideline is available in our formula cookbook, which we expect contributors to read before submitting PRs (as it is in the PR checklist).

As for the feature you proposed, do note that CI in homebrew/core does support automatic squashing when merging PRs labelled autosquash; see Homebrew/homebrew-core#152682 for example, where the PR is replaced by Homebrew/homebrew-core#152900 at the end. The failing CI check is only to highlight the commit style violation; it does not stop the PR from getting approved and merged ultimately. (If, however, what you proposed is to squash the commits on every push, then that is not implemented.)

Despite the autosquash workflow already being implemented, we still often tell contributors to squash the commits themselves, because this makes sure they understand this commit format when they continue to contribute in the future, and also saves CI resources since autosquash requires another round of CI on the squashed commit. So, I would argue that this is not "friction", but instead to guide contributors to contribute better.

I'm going to close this issue as this repository is for issues in in brew itself. But feel free to continue the discussion in our discussion forum.

[0xdevalias]

we enforce the "one formula per commit; one commit per formula" rule, so modifications to a formula needs to be squashed into one commit in every PR

None of which is prevent by squash merging PR's..?

This ensures tidiness in the Git log and can be useful when one needs to browse the history of a file.

Which is all still maintained by squash merging PR's.

As for the feature you proposed, do note that CI in homebrew/core does support automatic squashing when merging PRs labelled autosquash; see Homebrew/homebrew-core#152682 for example, where the PR is replaced by Homebrew/homebrew-core#152900 at the end.

I'm not sure if that is broken, or takes a very long time to kick in, or what; but it certainly didn't seem to happen on this PR; given the request for manual squashing/etc:

• goresym 2.6.3 (new formula) homebrew-core#155716 (comment)

The failing CI check is only to highlight the commit style violation; it does not stop the PR from getting approved and merged ultimately.

Fair enough. Though that's definitely not how it seemed to come across in the above linked PR.

(If, however, what you proposed is to squash the commits on every push, then that is not implemented.)

I wasn't proposing that; that wouldn't be a good user experience.

Despite the autosquash workflow already being implemented, we still often tell contributors to squash the commits themselves, because this makes sure they understand this commit format when they continue to contribute in the future, and also saves CI resources since autosquash requires another round of CI on the squashed commit. So, I would argue that this is not "friction", but instead to guide contributors to contribute better.

We can agree to disagree on this one. You're basically trading off some cheap/simple compute time against very real world human time (not just for me as someone contributing, but for the maintainers who are wasting their time explaining things that could be very easily automated away); and in doing so, making it more painful for a contributor to contribute; thus negatively reinforcing them not to bother. Not a good policy/choice tbh.

This has once more reminded me why I've tended to avoid spending my time contributing to homebrew; so thanks for that :)

[ZhongRuoyu]

None of which is prevent by squash merging PR's..?
Which is all still maintained by squash merging PR's.
I'm not sure if that is broken, or takes a very long time to kick in, or what; but it certainly didn't seem to happen on this PR; given the request for manual squashing/etc:

"Squash merging PR's" is working fine, it only happens at the end when a maintainer approves your PR (and also initiates a merge, for PRs adding new formulae). The "merge" has not happened to your PR yet.

Though that's definitely not how it seemed to come across in the above linked PR.

Because automated autosquash, compared to a maintainer requesting manual squashing, is less effective in making sure contributors understand the commit style and do it right in the future.

I wasn't proposing that; that wouldn't be a good user experience.

Yes, I agree.

You're basically trading off some cheap/simple compute time against very real world human time

Compute time can be simple on some PR, but extremely expensive on some others; it is, in general, valuable to us. CI on some important formulae (e.g. go and rust) can take hours or even days to complete. We definitely don't want to waste CI time by running everything again after autosquashing.

By saying this, however, I am not suggesting that we will reject PRs that require autosquash. Contributions are still welcomed, but more welcomed and appreciated when they come in the right format.

[MikeMcQuaid]

This has once more reminded me why I've tended to avoid spending my time contributing to homebrew; so thanks for that :)

@0xdevalias This sort of commentary is not necessary or appreciated. @ZhongRuoyu has provided a fairly comprehensive response to your issue. We welcome help making Homebrew better but these interactions do not help Homebrew in their current format and tone.

[0xdevalias]

This sort of commentary is not necessary or appreciated

@MikeMcQuaid I already wrote a longer response to this on the other issue (Ref), so I won't waste either of our time in repeating it here again.

[0xdevalias]

@ZhongRuoyu translated through ChatGPT to avoid 'tone':

Thank you for your detailed explanations and the context you provided. While I have a different perspective on the balance between manual processes and automation (seemingly aligned to this article), I understand and respect the project's current approach. Given our differing views, I think it's best to avoid further discussion on this topic to respect both our time commitments.