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.

Sign up for GitHub

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

Jump to bottom

Add pre-commit #2744

Merged
merged 15 commits into from
Aug 20, 2023
Merged

Add pre-commit #2744

Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7210ba5
Add pre-commit
CoolCat467 Aug 7, 2023
eec0dfa
Merge branch 'master' into add-pre-commit
CoolCat467 Aug 7, 2023
25e0281
Remove python version specifier from black
CoolCat467 Aug 8, 2023
b61ba1a
Merge branch 'master' into add-pre-commit
CoolCat467 Aug 8, 2023
0eaaaa3
Add codespell
CoolCat467 Aug 10, 2023
eee0795
Merge branch 'python-trio:master' into add-pre-commit
CoolCat467 Aug 10, 2023
caa68e7
No more skips
CoolCat467 Aug 10, 2023
d8d6a2d
Run pre-commit on all files and fix issues
CoolCat467 Aug 13, 2023
f7aede8
Merge branch 'add-pre-commit' of https://github.com/CoolCat467/trio i…
CoolCat467 Aug 13, 2023
0c05130
Merge branch 'master' into add-pre-commit
CoolCat467 Aug 13, 2023
9f00abf
Revert docs changes and ignore docs formatting in the future
CoolCat467 Aug 14, 2023
1d8ab83
Merge branch 'master' into add-pre-commit
CoolCat467 Aug 14, 2023
316838a
Disable black and isort autoformatting in CI
CoolCat467 Aug 15, 2023
6aaec43
Fix typo
CoolCat467 Aug 18, 2023
9978f6e
Merge branch 'master' into add-pre-commit
CoolCat467 Aug 18, 2023
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
36 changes: 36 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-toml
- id: check-merge-conflict
- id: mixed-line-ending
- id: check-case-conflict
- repo: https://github.com/psf/black
rev: 23.7.0
hooks:
- id: black
language_version: python3.8
CoolCat467 marked this conversation as resolved.
Show resolved Hide resolved
- repo: https://github.com/pycqa/isort
rev: 5.12.0
hooks:
- id: isort
- repo: https://github.com/pycqa/flake8
rev: 6.1.0
hooks:
- id: flake8
additional_dependencies:
- "flake8-pyproject==1.2.3"
CoolCat467 marked this conversation as resolved.
Show resolved Hide resolved
types: [file]
types_or: [python, pyi]

ci:
autofix_commit_msg: "[pre-commit.ci] auto fixes from pre-commit.com hooks"
autofix_prs: true
autoupdate_commit_msg: "[pre-commit.ci] pre-commit autoupdate"
autoupdate_schedule: weekly
skip: [flake8]
submodules: false
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want pre-commit CI (other than maybe for updating the pre-commit hooks). We're intentionally not pushing commits based on what autoformatters want since that might interfere with an WIP PR, for instance.

Copy link
Member

Choose a reason for hiding this comment

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

Running Black on WIP PR's should not be a problem. isort could in theory be a problem, but I suspect it'd help more often than it hurts - and in any case it doesn't modify your local repo and you can review the commit before isort does its thing if you want.

But it can definitely be messy, it's probably a tradeoff between experienced contributors (that will have taken the decision whether to run the pre-commit hooks locally) vs new contributors (that might struggle with setting up tooling and will waste time doing several CI runs until stuff passes) and people maybe having to review unformatted code.

Copy link
Member

Choose a reason for hiding this comment

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

2023-08-08T13:55:07,201929064+02:00
case in point 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I keep using the web editor for small changes :(

I do think for larger PRs changing the code from under their feet is a bit annoying.

Copy link
Member

Choose a reason for hiding this comment

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

using the web editor seems like a strong case in favor of it. Leaving it off at the beginning seems reasonable regardless just to make sure it works and all, and then we can try toggling it later