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

[ENH] Find alternative to pre-commit-rust #810

Open
andygrove opened this issue Sep 27, 2022 · 5 comments
Open

[ENH] Find alternative to pre-commit-rust #810

andygrove opened this issue Sep 27, 2022 · 5 comments
Labels
enhancement New feature or request needs triage Awaiting triage by a dask-sql maintainer

Comments

@andygrove
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I would like the pre-commit hook to run cargo test and cargo +nightly fmt so that we can format imports. These features are not implemented in pre-commit-rust, and the project seems to be unmaintained (last commit is more than 2 years ago).

There is a PR for supporting cargo test (doublify/pre-commit-rust#25) and an issue for supporting +nightly (doublify/pre-commit-rust#15).

Describe the solution you'd like
One option would be to fork the repo. I don't know what other solutions exist for the pre-commit tool we are using.

Describe alternatives you've considered
None

Additional context
None

@andygrove andygrove added enhancement New feature or request needs triage Awaiting triage by a dask-sql maintainer labels Sep 27, 2022
@andygrove
Copy link
Contributor Author

@charlesbluca fyi

@charlesbluca
Copy link
Collaborator

Thanks for the ping @andygrove - agree that an alternative to pre-commit-rust would allow us to have a lot more flexibility in terms of running cargo commands as part of CI, and I don't imagine it being too difficult.

The biggest blocker I foresee here (which I've been discussing with @ayushdg) is switching over our CI and documentation to encourage the use of rustup over the conda Rust package, so that users have access to the same pre-commit environment that we are using in CI (which presumably would include nightly Rust) - I can start looking into that

@eirnym
Copy link

eirnym commented May 3, 2024

@andygrove if you test as well with nightly toolchain, it's easier to set specfic toolchain as default.

@charlesbluca
Copy link
Collaborator

This issue is pretty outdated now, the solution we ended up going with here was encouraging direct install of rustup rather than the conda packages and then using +nightly for the import formatting:

There is still the issue of pre-commit-rust being unmaintained though, so it might be worth moving some of our remaining pre-commit checks either to a fork of the repo or to local hooks.

@eirnym
Copy link

eirnym commented May 8, 2024

I'd like to create pre-commit-rust from the scratch. May I invite you to discuss requirements?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs triage Awaiting triage by a dask-sql maintainer
Projects
None yet
Development

No branches or pull requests

3 participants