-
Notifications
You must be signed in to change notification settings - Fork 275
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
set up cargo-deny #89
Conversation
👷 Deploy Preview for apollo-router-docs processing. 🔨 Explore the source changes: e50d855 🔍 Inspect the deploy log: https://app.netlify.com/sites/apollo-router-docs/deploys/6184f38241d2e1000889804c |
.circleci/config.yml
Outdated
cargo xtask check | ||
- run: | ||
command: > | ||
cargo xtask test | ||
cargo xtask lint |
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.
check, lint.... 🤔 should it really be 2 different commands?
I think we should keep lint
and move your check there. You can exclude windows from the test there too using #[cfg(not(windows))]
.
Check would be fine too if it didn't collide with "cargo check" which means checking if it compiles, not linting. (counter intuitive for rust developers)
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.
It can indeed be a bit confusing! Let's try to think of a name that conveys we're making sure the codebase complies to our standards, license and fmt and stylewise.
wdyt?
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.
Looking at #147 and this, i think this calls for a team discussion about what xtask does and what the CI does. I'm opening an issue or maybe a discussion ?
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.
just opened #160 so we dont forget about it!
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.
Purrfect 😸
merging this when ci passes! |
Ok ci doesn't pass anymore 😓 fixing this! |
…er-core. added it to the release checklist
fixes #37
This commit sets up cargo-deny with the following settings:
RUSTSEC-2020-0159
(pending The call tolocaltime_r
may be unsound chronotope/chrono#499 to close somehow)"Apache-2.0", "Apache-2.0 WITH LLVM-exception", "BSD-2-Clause", "BSD-3-Clause", "CC0-1.0", "LicenseRef-ELv2", "ISC", "MIT",
.LicenseRef-ELv2
is a placeholder while we're waiting for a SPDX identifier.apollographql
andopentelemetry
github repositories only.This commit also adds
cargo-deny -L error check
on Linux and OSX in CI. Note it doesn't run on windows, possibly because of an environment variable issue. However this is fine given cargo-deny checks for every targets by default.