-
Notifications
You must be signed in to change notification settings - Fork 61
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 rapids-dependency-file-generator pre-commmit hook #682
Conversation
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.
@jameslamb Also FYI: rapidsai/shared-workflows#46 (comment)
There's a list of a few repos needing this update.
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.
Also we can disable the dependency check from the checks
job now that it's covered by style checks.
Like this in .github/workflows/pr.yaml
:
checks:
secrets: inherit
uses: rapidsai/shared-action-workflows/.github/workflows/checks.yaml@branch-24.04
with:
enable_check_generated_files: false
Regarding adding |
Yes. After we add pre-commit hooks we can change the default to false and later remove that option. |
There's a list of a few repos needing this update. Thanks! Hadn't seen that before! I can make those changes in those other repos right now, shouldn't take that long.
done in 4cf17d3 |
This reverts commit 4cf17d3.
On second thought... I just reverted that. @bdice instead of checking in configuration like this in all the repos: with:
enable_check_generated_files: false I think it'd be preferable to just add those pre-commit hooks to the remaining repos, and then remove the That'd avoid the need to come back and do another round of PRs removing the What do you think? |
That is smarter. Do that! |
Yep thinking the same thing. We can apply those cycles to some other goal |
This didn't happen. I did search for which ones need it and summarized that at rapidsai/shared-workflows#46 (comment). |
/merge |
Thanks all! 🙏 |
Proposes adding a pre-commit hook to run
rapids-dependency-file-generator
.With that change, local
pre-commit
and CI will raise an error if changes have been made independencies.yaml
that would affect any other files checked into the repo, like the conda envs in https://github.com/rapidsai/cucim/tree/branch-24.02/conda/environments.How I tested this
Changed a
cmake
version constraint independencies.yaml
.Ran
pre-commit run --all-files
.Observed the expected failure.
Reverted that change, ran
pre-commit run --all-files
again, saw everything pass.