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

Add pre-commit plus a GItHub Action to run it #776

Merged
merged 4 commits into from
Dec 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/precommit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# https://pre-commit.com
# This GitHub Action assumes that the repo contains a valid .pre-commit-config.yaml file.
# Using pre-commit.ci is even better that using GitHub Actions for pre-commit.
name: pre-commit
on:
pull_request:
branches: [master]
push:
workflow_dispatch:
jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: 3.x
- run: pip install pre-commit
cclauss marked this conversation as resolved.
Show resolved Hide resolved
- run: pre-commit --version
- run: pre-commit install
cclauss marked this conversation as resolved.
Show resolved Hide resolved
- run: pre-commit run --all-files
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/setup-python@v5
with:
python-version: 3.x
- run: pip install pre-commit
- run: pre-commit --version
- run: pre-commit install
- run: pre-commit run --all-files
- uses: actions/setup-python@v5
with:
python-version: 3
- uses: pre-commit/[email protected]

Copy link
Contributor Author

@cclauss cclauss Dec 23, 2023

Choose a reason for hiding this comment

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

https://github.com/pre-commit/action is in maintenance-only mode and will not be accepting new features so I tend not to use it. pre-commit.ci is awesome.

Copy link
Contributor

@akx akx Dec 23, 2023

Choose a reason for hiding this comment

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

I know it's in maintenance-only mode, but it works – it's also just, well, 20 lines.

I don't prefer pre-commit.ci because it's a third-party dependency and (AFAIU) an one-man project (and for projects where "auto-fix PRs" is enabled, they tend to mess with my workflow).

Copy link
Contributor Author

@cclauss cclauss Dec 23, 2023

Choose a reason for hiding this comment

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

pre-commit.ci is run by the same person who led the development of pre-commit.

If auto-fix PRs are messing up your workflow then you do not have pre-commit properly installed locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cclauss Well, for a repo like this where every commit should have a --signoff to adhere to a CLA, the pre-commit commit wouldn't be appropriate, etc. 😁

39 changes: 39 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Learn more about this config here: https://pre-commit.com/

# To enable these pre-commit hooks run:
# `brew install pre-commit` or `python3 -m pip install pre-commit`
# Then in the project root directory run `pre-commit install`

# default_language_version:
# python: python3
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
hooks:
- id: check-builtin-literals
# - id: check-executables-have-shebangs
# - id: check-shebang-scripts-are-executable
- id: check-toml
- id: check-xml
- id: check-yaml
# - id: detect-private-key
# - id: end-of-file-fixer
# - id: mixed-line-ending
# - id: trailing-whitespace

#- repo: https://github.com/codespell-project/codespell
# rev: v2.2.6
# hooks:
# - id: codespell # See pyproject.toml for args
# additional_dependencies:
# - tomli

- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.1.9
hooks:
- id: ruff # See pyproject.toml for args

- repo: https://github.com/abravalheri/validate-pyproject
rev: v0.15
hooks:
- id: validate-pyproject
21 changes: 10 additions & 11 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
[tool.ruff]
line-length = 167

exclude = ["test/lib/python/*"]
extend-select = [
"S", # Bandit
"E9",
"F63",
"F7",
"F82",
"I",
]
ignore = [
"E402", # TODO: enable
"E711", # TODO: enable
"E711", # TODO: enable
"E712", # TODO: enable
"E721", # TODO: enable
"S101", # TODO: enable
"S110", # TODO: enable
"S324", # TODO: enable
]
extend-select = [
"S", # Bandit
"E9",
"F63",
"F7",
"F82",
"I",
]
line-length = 167

[tool.ruff.per-file-ignores]
"{test,tests,examples}/**/*.py" = [
Expand Down
Loading