be more consistent about workspace members #5921
Conversation
.github/buildomat/jobs/clippy.sh
Outdated
| @@ -30,4 +30,4 @@ ptime -m bash ./tools/install_builder_prerequisites.sh -y | |||
| banner clippy | |||
| export CARGO_INCREMENTAL=0 | |||
| ptime -m cargo xtask clippy | |||
| ptime -m cargo doc | |||
| RUSTDOCFLAGS="-Dwarnings" ptime -m cargo doc --workspace --keep-going | |||
There was a problem hiding this comment.
Do we actually want the --keep-going flag here? Wouldn't that increase CI times for these workflows?
(I agree that it would be convenient locally, but I figured we'd want the CI to return any failures ASAP)
There was a problem hiding this comment.
It would increase the amount of time until GitHub tells you the job failed, yeah.
I don't know whether it's better to have the red X show up faster or have the logs show as many errors as possible (not necessarily all errors).
There was a problem hiding this comment.
(If you don't have a preference either, I'll hedge towards not changing it and drop it from this PR.)
There was a problem hiding this comment.
I think i have a mild preference for the default, aka, the "stop early" option, just because anything that gets results back from CI faster seems like a strong default unless we're convinced otherwise.
dev-tools/xtask/src/clippy.rs
Outdated
| @@ -42,6 +42,8 @@ pub fn run_cmd(args: ClippyArgs) -> Result<()> { | |||
| command | |||
| // Make sure we check everything. | |||
| .arg("--all-targets") | |||
| .arg("--workspace") | |||
| .arg("--keep-going") | |||
There was a problem hiding this comment.
Same comment as above, re: CI time
We have reasons for excluding some packages from the default workspace members, but there was nothing checking that newly-added packages were added to both. I added an additional check to
cargo xtask check-workspace-depsto ensure thatdefault-membersis equal tomembersminus xtask and end-to-end-tests.This had the effect that the tests in
api_identitywere not being run in CI!Add
--workspacetocargo clippyandcargo docin CI to run checks across the entire tree. (Also added--keep-goingso that CI shows as many errors as possible.)Fix clippy lints / doc errors that were not being caught in the previous setup.