Clean-up + add hygiene checks - #463
Conversation
|
|
||
| # Install uv for quicker package installations (installed to ~/.local/bin with --user) | ||
| RUN python -m pip install --user uv==0.9.29 | ||
| ENV UV_NO_CACHE=1 |
There was a problem hiding this comment.
I understand not wanting to cache in the docker build phase to avoid releasing extra blobs, but I think it may be inconvenient for the user to find that uv doesn't cache (given that it is such a useful feature of uv!). Can we instead set this on each run command (or unset at the end of the build)?
There was a problem hiding this comment.
Hmm, I'm setting the environment variable during the workshop stage. It should be wiped when we call the second FROM, i.e. when we create the user's workspace. Is there something I'm missing?
| python -m pip install --user --no-cache-dir pre-commit==4.5.1 reuse==6.2.0 | ||
| echo "$HOME/.local/bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Run whole-repo whitespace check |
There was a problem hiding this comment.
Why can't we just run the whole set in one pre-commit command?
There was a problem hiding this comment.
Because of the reuse annotate hooks; they should only apply to changed files. Otherwise copyright headers of files that haven't actually been touched will be updated. By itself this shouldn't be a problem since we're not committing files in the CI but it will still cause the hygiene CI to fail (erroneously)
There was a problem hiding this comment.
Turns out there is a better solution for this (yay!). The idea is to add a custom stage to everything by default (I've gone for manual) then trigger all hooks with that stage when running hooks in the CI. I've updated the PR to this effect.
jondea
left a comment
jondea
left a comment
There was a problem hiding this comment.
Generally looks great, thanks! I just had some minor comments.
b551afb to
cd3f193
Compare
cd3f193 to
a7418e2
Compare
2 participants
PR Summary
Adds a new hygiene CI workflow and expands pre-commit coverage.
.github/workflows/hygiene.ymlto run changed-file pre-commit hooks, safe whole-repo hygiene checks, whole-reporeuse lint..pre-commit-config.yamlwith EOF, line-ending, JSON/YAML, case/symlink, large-file, executable/shebang, andruff-checkhooks, with comments for non-obvious behavior.Dockerfiles to avoidpip/uvcache directories in final images.rufffindings in PyTorch/TensorFlow examples.Note
I propose that we hold off until #458 is merged before we consider merging this. We could potentially even wait until after the 26.06 release.