Skip to content
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

Setup pre-commit.ci #124

Open
jakirkham opened this issue Dec 5, 2024 · 3 comments
Open

Setup pre-commit.ci #124

jakirkham opened this issue Dec 5, 2024 · 3 comments

Comments

@jakirkham
Copy link
Member

To make it easier to use pre-commit, it would be great to have https://pre-commit.ci configured for all RAPIDS projects. We can also make this a requirement before running other CI

Additionally we could leverage pre-commit.ci to help us fix issues with its bot. Though would recommend that disable autofixing. Otherwise the bot pushes changes into PRs fairly aggressively. It would be better to have users request fixes from the bot. This can be done in .pre-commit-config.yaml like so

ci:
  autofix_prs: false

We may also want to add an exception in our own CI configuration to allow commits from the pre-commit bot when considering whether to run CI

@vyasr
Copy link
Contributor

vyasr commented Dec 5, 2024

There are a number of reasons why we haven't been able to use it in the past and have opted for running pre-commit ourselves. I can link you to threads, please feel free to let us know if any of those reasons have materially changed. I'd love to use the service if possible.

@jakirkham
Copy link
Member Author

jakirkham commented Dec 5, 2024

Links would be helpful

Or simply a comment here with a list of outstanding issues. Perhaps we can use that as a way to close them out one-by-one so as to unblock this issue

@bdice
Copy link
Contributor

bdice commented Dec 5, 2024

We had some offline discussions about this. CCCL and cuCollections are using pre-commit.ci but there are some limitations.

  • We can't run our doxygen checks through pre-commit.ci because it requires a package version that is hard to install through any means supported on pre-commit.ci (but easy via apt or conda -- and the pre-commit "conda" hooks are not viable for several reasons, including a hard requirement on having conda and being willing to put up with very slow initial pre-commit runs while conda packages install)
  • Getting the bot to push to our PRs is apparently possible (CCCL does it) but it introduces an untrusted commit, so we have to comment /ok to test for all subsequent commits. This is nonviable for autofixes, only manually triggered fixes (by a comment like pre-commit.ci autofix)

Nonetheless, we are in agreement that this is worth trying. I will open a PR for RMM and request for ops to install the application in the rapidsai org so we can try it out.

rapids-bot bot pushed a commit to rapidsai/rmm that referenced this issue Dec 9, 2024
This PR adds configuration for pre-commit.ci, reformats the pre-commit config file, and updates pre-commit hooks.

See rapidsai/build-planning#124 for the motivation.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Matthew Murray (https://github.com/Matt711)
  - Jake Awe (https://github.com/AyodeAwe)

URL: #1746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants