Skip to content

Add pre-commit including config and GitHub workflow#563

Merged
m1yag1 merged 2 commits intopyslackers:mainfrom
josephgruber:add-pre-commit
Sep 23, 2025
Merged

Add pre-commit including config and GitHub workflow#563
m1yag1 merged 2 commits intopyslackers:mainfrom
josephgruber:add-pre-commit

Conversation

@josephgruber
Copy link
Contributor

Relevant Issue & Description

https://pythondev.slack.com/archives/C2FMLUBEU/p1757474340679269

This PR introduces pre-commit to automatically enforce code formatting and linting standards across the project, especially related to Ruff.

Pre-commit Setup

  • Added .pre-commit-config.yaml with built-in pre-commit hooks including:
    • File validation (large files, merge conflicts, JSON/TOML/YAML syntax)
    • Security checks (private key detection)
    • Code formatting (end-of-file-fixer, trailing-whitespace)
  • Added ruff format and lint (check) hooks via the official hook repo

Enforced linting and formatting via GitHub Workflow

  • Added a GitHub Actions workflow (.github/workflows/pre-commit.yml) to run pre-commit on all pull requests

Project Configuration Updates

  • Updated Python version requirement from >=3.8 to >=3.12 in pyproject.toml
  • Added pre-commit>=4.3.0 to lint dependencies
  • Updated Ruff version to >=0.13.0
  • Updated CONTRIBUTING.md with pre-commit installation and usage instructions

File Formatting

  • SVG and CSS files include trailing newlines (per best practices) via the pre-commit hook

Screenshots, if applicable

Screenshot_20250912_002660

@josephgruber
Copy link
Contributor Author

And via the new workflow:

Screenshot_20250912_002664

Copy link

@Quidge Quidge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes but +1 on this add (and how you've added it)!

- uses: actions/setup-python@v6
with:
python-version: "3.12"
- uses: pre-commit/action@v3.0.1
Copy link

@Quidge Quidge Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rolled without using the dedicated pre-commit action before, but something I've added in my own CI is the --show-diff-on-failure flag. It will show a git diff on a failure so you can see the issue in the CI readout. Unsure if the dedicated pre-commit CI action is already doing that under the hood (should be easy enough to check source for that action and confirm if they're using it or not; they likely also have a way to pass arbitrary flags).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +22 to +25
- id: ruff-check
args: [--exit-non-zero-on-fix]
- id: ruff-format
args: [--exit-non-zero-on-fix]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't exit-non-zero the default? And/or, when pre-commit runs, if a delta is detected, doesn't the delta itself cause pre-commit to trip a failure anyways?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But Ruff can also auto-magically fix some formatting and linting issues. In which case, it exits with a zero status code from what I've seen. We don't have the fix attribute set so it won't auto-fix any issues currently but I'd still suggest we leave this argument on, in case fix is enabled in the future.

@m1yag1 m1yag1 merged commit a7a0dae into pyslackers:main Sep 23, 2025
2 checks passed
@josephgruber josephgruber deleted the add-pre-commit branch September 23, 2025 13:37
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