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

Consider using pre-commit.ci for style checking in CI #195

Closed
flferretti opened this issue Jul 4, 2024 · 5 comments · Fixed by #197
Closed

Consider using pre-commit.ci for style checking in CI #195

flferretti opened this issue Jul 4, 2024 · 5 comments · Fixed by #197
Assignees

Comments

@flferretti
Copy link
Collaborator

By using pre-commit.ci, we could eliminate the file related to the style CI configuration, e.g. .github/workflows/style.yml and substitute it with something that leverages the already present .pre-commit-config.yml and the configuration from the pyproject.toml.

This would allow us to add additional checks (see https://github.com/pre-commit/pre-commit-hooks), especially the check-added-large-files, which would prevent the user to commit the pixi.lock pulled from lfs, and the others checks already present in the pre-commit configuration.

@flferretti flferretti changed the title Consider adding pre-commit.ci instead for style checking in CI Consider adding pre-commit.ci for style checking in CI Jul 4, 2024
@flferretti flferretti changed the title Consider adding pre-commit.ci for style checking in CI Consider using pre-commit.ci for style checking in CI Jul 4, 2024
@flferretti flferretti self-assigned this Jul 4, 2024
@diegoferigo
Copy link
Member

diegoferigo commented Jul 5, 2024

Running the pre-commit check also on CI would be great! About the style checks (isort and black), I would just double check about the following:

  • We can constrain the version of black as we currently do. In the past, new major releases (once per year, usually around January) introduced breaking changes. I prefer to bump manually the black pinning.
  • Right now the CI provides a nicely formatted diff in case there is a style mismatch. Before moving to ruff, let's be sure to get something comparable.

@flferretti
Copy link
Collaborator Author

The formatting packages are already constrained in the .pre-commit-config.yml for both black:

rev: 24.2.0

and isort:

rev: 5.13.2

Do you prefer to have an error raised with the diff or just let the CI format the commited files?

@diegoferigo
Copy link
Member

Nope the pinning looks good. Can we match it with what specified in pyproject.toml? Considering the update policies, I guess that pinning the major versions would suffice.

@flferretti
Copy link
Collaborator Author

The pre-commit configuration needs an exact tag or commit SHA. Can we stick with the current configuration since it matches:

# From pyproject.toml in #193 
"black[jupyter] ~= 24.0"

?

@diegoferigo
Copy link
Member

Yes that sounds good, I thought it could handle less strict versioning.

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 a pull request may close this issue.

2 participants