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

set up cargo-deny #89

Merged
merged 8 commits into from
Nov 18, 2021
Merged

set up cargo-deny #89

merged 8 commits into from
Nov 18, 2021

Conversation

o0Ignition0o
Copy link
Contributor

@o0Ignition0o o0Ignition0o commented Nov 5, 2021

fixes #37

This commit sets up cargo-deny with the following settings:

  • Security advisories: warn for unmaintained and yanked crates. deny CVEs except RUSTSEC-2020-0159 (pending The call to localtime_r may be unsound chronotope/chrono#499 to close somehow)
  • Licenses: Allow ELv2 compatible licenses: "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.
  • Sources: Explicitly allow references to apollographql and opentelemetry github repositories only.
  • Bans: No crates are banned as of yet

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.

@netlify
Copy link

netlify bot commented Nov 5, 2021

👷 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

@o0Ignition0o o0Ignition0o marked this pull request as ready for review November 5, 2021 15:56
@o0Ignition0o o0Ignition0o requested review from cecton, abernix, BrynCooke and Geal and removed request for abernix November 5, 2021 15:56
Comment on lines 218 to 221
cargo xtask check
- run:
command: >
cargo xtask test
cargo xtask lint
Copy link
Contributor

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)

Copy link
Contributor Author

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?

Copy link
Contributor Author

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 ?

Copy link
Contributor Author

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!

Copy link
Contributor

Choose a reason for hiding this comment

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

Purrfect 😸

@o0Ignition0o
Copy link
Contributor Author

merging this when ci passes!

@o0Ignition0o
Copy link
Contributor Author

Ok ci doesn't pass anymore 😓 fixing this!

@o0Ignition0o o0Ignition0o merged commit 222f068 into main Nov 18, 2021
@o0Ignition0o o0Ignition0o deleted the igni/cargo_deny branch November 18, 2021 16:36
@o0Ignition0o o0Ignition0o self-assigned this Mar 25, 2022
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.

Setup cargo-deny
2 participants