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 clippy and rustfmt to Rust build #486

Closed
wants to merge 2 commits into from

Conversation

samstokes
Copy link
Contributor

As discussed in #420 this adds clippy (style checks) and rustfmt (code formatting check) to the Rust build.

Details:

  • following OCamlformat all of the things! #477 this checks formatting only in CI, not in dev builds. cargo fmt is actually very quick (sub-second on my machine) so it'd be entirely feasible to run it in dev too, but the dev use case is better served by editor integration, and anyway for quick feedback I'd rather see if my code compiles and passes tests before fixing possible formatting errors.
  • this does run clippy checks in dev (after tests), but doesn't fail the build for warnings. In CI, warnings do fail the build.
  • for some reason clippy's default behaviour is to style-check every single crate that your code depends on before actually checking the code you wrote. I can't work out how to turn that off. On the plus side I think build caching should mean it only does that occasionally.

N.B. this PR is based on #478 (for the sake of having some non-trivial code to check) - once that merges I'll rebase this branch and change the base of this PR to master.

@samstokes
Copy link
Contributor Author

The CI failure is in the OCaml build, and I think is because #478 needs rebasing onto master - hoping the rebase will fix both builds.

@IanConnolly
Copy link
Contributor

IanConnolly commented Dec 7, 2018

or some reason clippy's default behaviour is to style-check every single crate that your code depends
on before actually checking the code you wrote. I can't work out how to turn that off. On the plus side
I think build caching should mean it only does that occasionally.`

IIRC it's not actually checking them, but it does need to build them as far as the HIR stage to get type information to be able to lint the stroller code.

Copy link
Contributor

@IanConnolly IanConnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! It can land whenever the other PR gets over the line

@samstokes samstokes force-pushed the sam/clippy-and-rustformat branch 2 times, most recently from d64034c to 9313840 Compare December 11, 2018 05:55
@pbiggar
Copy link
Member

pbiggar commented Dec 14, 2018

Apologies for closing this, was unintentional, no idea how it happened.

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

Successfully merging this pull request may close these issues.

3 participants