-
Notifications
You must be signed in to change notification settings - Fork 38
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 remaining pre-commit-hooks #430
Conversation
c0dac9c
to
25f9bca
Compare
Looks like you need to define the xml file type. |
enabled: false | ||
output: pass_fail | ||
read_output_from: stdout | ||
run: check-added-large-files ${target} |
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.
Can we add python3
to the front of these if appropriate? Shebangs don't work for these executions on Windows.
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.
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.
False alarm. This was previously WAI, the files are .exe on Windows and inside of Scripts/
, so no need for the python3 bit.
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.
Updated
.github/workflows/pr.yaml
Outdated
linter_tests_linux: | ||
name: Linter Tests Linux | ||
runs-on: public-amd64-2xlarge | ||
needs: detect_changes | ||
if: | ||
needs.detect_changes.outputs.linters == 'true' || needs.detect_changes.outputs.all-linters == |
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.
If we end up keeping this, can we merge it back into the matrix and add a new field to the object for the correct runner name?
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.
LGTM
No description provided.