Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Maintenance] Run pre-commit in CI and update periodically #349

Open
EwoutH opened this issue Oct 26, 2023 · 4 comments
Open

[Maintenance] Run pre-commit in CI and update periodically #349

EwoutH opened this issue Oct 26, 2023 · 4 comments

Comments

@EwoutH
Copy link
Contributor

EwoutH commented Oct 26, 2023

Currently there is a nice pre-commit configuration, in .pre-commit-config.yaml, but there are two problems:

  • It isn't run and enforced in CI, so currently it doesn't do anything.
  • The file isn't periodically updated, so most pre-commit actions are over a year old.

Currently running pre-commit.ci results in 123 files changed - all with very minor things like whitespaces/lines and import orders. This why pre-commit only works if it's ran on every commit (in CI).

There are different approaches on this, but by far the most convenient (at least that I know of) is using pre-commit.ci. It takes care of:

  • Running pre-commit in CI, and warning if any tests fail
  • Updating the pre-commit configuration periodically (daily/weekly/monthly)
  • (optionally) directly applying fixes to open PRs (you can enable or disable this)

If this sounds useful, the first step is enabling pre-commit.ci for this repo.

@maartenbreddels
Copy link
Contributor

I'm not sure if I like another service. But I do like running pre commit on everything in CI: #225

That for some reasons failed (my guess was isort+black order / and external/internal library detection).

I wonder if we switch to ruff we can avoid the issue in #225, do you think if that works that #225 is a nice middleground solution?

@EwoutH
Copy link
Contributor Author

EwoutH commented Oct 30, 2023

Personally I don't have experience with Ruff yet, but I hear a lot of nice stories about it.

I see that there is a ruff pre-commit hook:

And it looks like they have proper Black compatibility: https://docs.astral.sh/ruff/formatter/#black-compatibility

So for that part, I think it should work.

But then:

Updating the pre-commit configuration periodically (daily/weekly/monthly)

For this we also need a solution. We could write custom workflow, but that would also need maintenance. Since Dependabot will likely never add support (dependabot/dependabot-core#1524), pre-commit.ci is the only properly maintained solution (I've used it for years in other libraries, its really nice).

@maartenbreddels
Copy link
Contributor

Updating the pre-commit configuration periodically (daily/weekly/monthly)

What is the benefit of that? That you are always up to date with the latest versions of all tools you use?

@EwoutH
Copy link
Contributor Author

EwoutH commented Oct 30, 2023

Exactly!

And that you can see if anything breaks if you update to the latest tools (since a PR is opened to update the tools, on which CI is ran).

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

No branches or pull requests

2 participants