ci: Run pre-commit checks in CI #176
Conversation
facebook-github-bot
added
the
CLA Signed
russellb
requested review from
ashwinb,
yanxi0830,
hardikjshah,
dltn and
raghotham
as code owners
|
You can find a passing test run of this here: https://github.com/russellb/llama-stack/actions/runs/11163763978 |
| @@ -21,11 +21,11 @@ ignore = | |||
| optional-ascii-coding = True | |||
| exclude = | |||
| ./.git, | |||
| ./docs | |||
| ./build | |||
| ./docs/*, | |||
.github/workflows/pre-commit.yml
Outdated
| pip install pre-commit | ||
|
|
||
| - name: Run pre-commit | ||
| run: pre-commit run --all-files |
There was a problem hiding this comment.
I don't think we should run it on all files because this means people will get stopped if someone committed something outside of this hook and now their PR will have to mirror all the fixes.
There was a problem hiding this comment.
That shouldn't happen, right? I think it's a fair expectation that anyone pushing directly to the repo is using the pre-commit hook.
If that were to happen anyway, they should get an email that this job failed because it will run any time commits are pushed, as well.
With that said, if you still want it to only apply to the diff instead of the full tree, I'll make the change. Just let me know.
There was a problem hiding this comment.
That shouldn't happen, right? I think it's a fair expectation that anyone pushing directly to the repo is using the pre-commit hook.
Yes that's the expectation but this happens for a bunch of reasons right now. Not everyone has the hooks set up and we are sort of slowly becoming more disciplined about every thing. Adding this right now might stop a bunch of folks in the tracks so to speak.
But I agree with you that this should be the norm. And we should make this happen sooner than later. For now though, let's make it only run on the files in the diff.
There was a problem hiding this comment.
sounds good! will update and comment again when it's done
There was a problem hiding this comment.
took a bit, but i got it working -- passing run on a test PR here: https://github.com/russellb/llama-stack/actions/runs/11168500406/job/31047043034?pr=1
There was a problem hiding this comment.
nevermind, it's failing here on this PR ...
There was a problem hiding this comment.
... ok, working for real this time
russellb
force-pushed
the
pre-commit-ci
branch
6 times, most recently
from
bcc8d09 to
37a7a02
Compare
Run the pre-commit checks in a github workflow to validate that a PR or a direct push to the repo does not introduce new errors. Signed-off-by: Russell Bryant <[email protected]>
The list of paths under `exclude` had a formatting error. Entries must be separated by commas. Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: Russell Bryant <[email protected]>
russellb
force-pushed
the
pre-commit-ci
branch
4 times, most recently
from
c5a7628 to
98f404a
Compare
Signed-off-by: Russell Bryant <[email protected]>
russellb
force-pushed
the
pre-commit-ci
branch
from
98f404a to
0515d5f
Compare
Closes #172
e6c3a4b ci: Run pre-commit checks in CI
adeee1e flake8: Fix config formatting error
commit e6c3a4b
Author: Russell Bryant [email protected]
Date: Thu Oct 3 13:52:24 2024 +0000
commit adeee1e
Author: Russell Bryant [email protected]
Date: Thu Oct 3 14:00:20 2024 +0000