-
Notifications
You must be signed in to change notification settings - Fork 223
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
use GitHub actions for linting #1616
Conversation
This reverts commit e825b3e.
.github/workflows/pre-commit.yml
Outdated
- uses: pre-commit/action@646c83fcd040023954eafda54b4db0192ce70507 | ||
with: ## let's skip to run for all the files | ||
extra_args: --help | ||
- id: files | ||
uses: jitterbit/get-changed-files@b17fbb00bdc0c0f63fcf166580804b4d2cdc2a42 | ||
- name: Configure PATH | ||
run: echo "${GITHUB_WORKSPACE}/.ci/scripts" >> $GITHUB_PATH | ||
- name: Precommit changes | ||
run: | | ||
for changed_file in ${{ steps.files.outputs.all }}; do | ||
pre-commit run --files "${changed_file}" | ||
done |
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 could not find a better way to run pre-commit
for the subset of changes.
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.
Honestly I'd be fine with running pre-commit
on everything. It should be very fast. But this also seems fine 🤷♂️
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.
Wouldn't this also work without the for loop?
pre-commit run --files ${{ steps.files.outputs.all }}
pre-commit run --files
can take a list of files. Not sure if we could run into some limit if the change affects hundreds of files, though (e.g. when Python 4.0 moves from spaces to tabs ;) )
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.
good point, I'll try it in a bit :)
🌐 Coverage report
|
Closed this PR, since it was much nicely done in #1658 :) |
What does this pull request do?
use GitHub actions for linting