Skip to content

Conversation

@nyurik
Copy link
Contributor

@nyurik nyurik commented Jul 23, 2025

DO NOT MERGE until #163 is merged

Once enabled, pre-commit CI will keep all PRs clean by automatically doing cargo fmt and other minor linting, without requiring users to re-submit their changes. Note that this is done by CI on the server, and not by user's own git hook.

Note that this used to be part of #83

@behdad
Copy link
Member

behdad commented Jul 23, 2025

This feels too magic for me. So, the commit I push is NOT the commit that shows up on the PR?

@nyurik
Copy link
Contributor Author

nyurik commented Jul 23, 2025

not exactly - take a look at

e.g.
image

As you can see - whenever a user submits a PR, if they forgot to cargo fmt the code or a few other minor simple spacing things, precommit automatically pushes another commit - that is very clear and easy to notice (and you get a notification) - indicating that a simple automatic step ran after your commit, and did the boring thing of code formatting.

This greatly improves the process for the casual contributors. Otherwise, if a user forgets to format, the contributor has to wait until CI completes, reports a simple annoying error, than the user has to re-submit (while swearing quietly), and only then get the "real" CI feedback. Very often projects do not even run CI for new contributors - which means the contribution is not even reviewed right away, and only when approved to run, the feedback is generated.

So to avoid all these papercuts, precommit just fixes the simple things. And as I mentioned - MapLibre (a very large project), and numerous other projects have had massive success with this - attracting casual and long term contributors.

@nyurik
Copy link
Contributor Author

nyurik commented Jul 23, 2025

P.S. this is very similar to the common process of the project maintainer fixing some simple things in a user's PR

@nyurik
Copy link
Contributor Author

nyurik commented Jul 23, 2025

P.P.S. This pattern is actually massively popular now - GitHub search returns 153,000 cases (and that's excluding forks!)

@khaledhosny
Copy link
Contributor

I personally hate formatting-only commit, they litter the commit history with needless noise.

@nyurik
Copy link
Contributor Author

nyurik commented Jul 23, 2025

@khaledhosny 100% agree - that's why all my projects require squash merge -- in the main settings page:

image

@nyurik
Copy link
Contributor Author

nyurik commented Jul 23, 2025

notice that most contributors keep adding to the PR additional commits - and only when things get merged, they get quashed into a single entry in the main history

Once enabled, pre-commit CI will keep all PRs clean by automatically doing `cargo fmt` and other minor linting, without requiring users to re-submit their changes. Note that this is done by CI on the server, and not by user's own git hook.

# Maintainer TODO

sign-in into https://pre-commit.ci/ and enable this repo for automatic PR validation (we had tons of success with this at MapLibre, and many other projects)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants