-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update .pre-commit-config.yaml #991
Conversation
Update isort version and repo
Remove node setup and node-version
Add env
a88a8d4
to
7e6bbe8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pre-commit skipping files issue should now be resolved with these changes.
@@ -1,6 +1,6 @@ | |||
# Configuration file for `pre-commit` tool. | |||
# Source: https://pre-commit.com/#pre-commit-configyaml---top-level | |||
exclude: "docs|node_modules|migrations|.git|.tox|.md|sample_setups/(external-setups|jsons)|tests/deprecated" | |||
exclude: "docs|node_modules|migrations|.git|.tox|README.md|sample_setups/(external-setups|jsons)|tests/deprecated" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lee1043 and @acordonez, I isolated the issue of pre-commit
skipping files to the .md
substring in the exclude
config. I'm not sure if pre-commit
can handle file extensions or not (.tox
is a not a file extension). I tried quickly Googling but didn't find any results.
I updated .md
to README.md
and now pre-commit
runs locally and on CI/CD. Now pre-commit run --all-files
picks up some issues that need to be fixed.
Let me know if pre-commit
is working for you locally as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomvothecoder this is also working locally for me now when I run pre-commit run --all-files
. Thanks for looking into this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomvothecoder thank you very much for figuring this out! At some point I added .md assuming it was for file extensions because I was annoyed by pre-commit complaining for extra space at the end of line in .md files. My bad. I appreciate you figured this out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acordonez and @lee1043 great and no problem!
Feel free to merge after you fix the new pre-commit
issues. You can rebase your dev branches on the latest main
to get pre-commit
working again.
pyproject.toml
Outdated
ignore_missing_imports = true | ||
warn_unused_ignores = true | ||
warn_redundant_casts = true | ||
warn_unused_configs = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved most configs from setup.cfg
to pyproject.toml
because pyproject.toml
is a more modern standard.
@@ -9,6 +9,10 @@ on: | |||
|
|||
workflow_dispatch: | |||
|
|||
env: | |||
CANCEL_OTHERS: true | |||
PATHS_IGNORE: '["**/README.rst", "**/docs/**", "**/ISSUE_TEMPLATE/**", "**/pull_request_template.md", "**/.vscode/**"]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PATHS_IGNORE: '["**/README.rst", "**/docs/**", "**/ISSUE_TEMPLATE/**", "**/pull_request_template.md", "**/.vscode/**"]' | |
PATHS_IGNORE: '["**/README.md "**/docs/**", "**/ISSUE_TEMPLATE/**", "**/pull_request_template.md", "**/.vscode/**"]' |
Update black version and isort repo following the xcdat (https://github.com/xCDAT/xcdat/blob/main/.pre-commit-config.yaml)