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

Add make targets for shellcheck, rustfmt, and a "test everything before I push" #6551

Open
davidMcneil opened this issue May 13, 2019 · 4 comments
Labels
Stale Type: Chore Issues for general code and infrastructure maintenance Type: Feature Issues that describe a new desired feature

Comments

@davidMcneil
Copy link
Contributor

It would be nice to have make targets that run test/shellcheck.sh, support/ci/rustfmt.sh, and one to test several things that are tested in the CI build. This would be useful to run before submitting a PR. The list of things to test could include things such as:

  • rustfmt
  • clippy
  • shellcheck
  • tests
@baumanj
Copy link
Contributor

baumanj commented May 20, 2019

I think this is a great idea!

Some things to keep in mind for implementing this:

rustfmt should be fairly straightforward. The main complication would be making sure it's running with the rust toolchain specified in RUSFMT_VERSION.

clippy for local runs needs to consider rust-lang/rust-clippy#2604, or else there will likely be false negatives due to the fact that code has already been built locally.

shellcheck can be run well locally, but the current script is oriented towards finding all potential shell files to check rather that running quickly. We could look into hybrid approaches, but I'd be careful that we don't make changes that would require newly added shell files to be manually added to a list to receive checking.

For rustfmt and shellcheck, I think the best approach is to have editor integration to run them while files are edited. Looking into pre-commit hooks may also be useful.

The las thing I'd say is that if we do this work, it would be particularly nice if we can provide a common interface for both linux and windows platforms. A lot of these sort of tasks have been codified in our Makefile, but that leaves out windows.

@dmccown
Copy link

dmccown commented May 24, 2019

@mwrock Thoughts on a way to get our Makefile to work across Windows and Linux?

@baumanj
Copy link
Contributor

baumanj commented May 24, 2019

I think we'd be better served by looking into writing cargo extension commands to do these tasks. Then we can implement them in rust and they're inherently cross-platform.

@raskchanky raskchanky self-assigned this Oct 11, 2019
@christophermaier christophermaier added the Type: Chore Issues for general code and infrastructure maintenance label Jul 24, 2020
@christophermaier christophermaier added Type: Feature Issues that describe a new desired feature and removed C-chore labels Jul 24, 2020
@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

@stale stale bot added the Stale label Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Type: Chore Issues for general code and infrastructure maintenance Type: Feature Issues that describe a new desired feature
Projects
None yet
Development

No branches or pull requests

6 participants