Skip to content

chore: align workspace with Rust best practices (edition 2024, MSRV CI, deps, profiles)#163

Merged
pofallon merged 1 commit into
mainfrom
chore/rust-best-practices
May 30, 2026
Merged

chore: align workspace with Rust best practices (edition 2024, MSRV CI, deps, profiles)#163
pofallon merged 1 commit into
mainfrom
chore/rust-best-practices

Conversation

@pofallon
Copy link
Copy Markdown
Contributor

Conformance pass against current Rust packaging/organization norms (Cargo Book, API guidelines, docs.rs/release tooling). Driven by a project-wide best-practices audit.

Edition & MSRV

  • Migrate both crates to edition 2024 + resolver = "3" (MSRV-aware resolver), bump rust-version to 1.85. Ran cargo fix --edition for the mechanical changes (match-ergonomics, import ordering); edition-2024 rustfmt accounts for most of the line count.
  • Add an MSRV CI job pinned to 1.85 (cargo check --workspace --all-targets --all-features --locked) so the declared MSRV is actually verified instead of aspirational.

unsafe_code: forbiddeny, with zero unsafe added

Edition 2024 makes std::env::set_var/remove_var unsafe. Rather than scatter #[allow(unsafe_code)], the env mutation is eliminated:

  • Production (the 2 sites): cli.rs threads the default log directive into a new init_with_redaction_and_directive instead of mutating RUST_LOG; docker_retry.rs replaces its env round-trip test seam with an atomic counter (also fixes a latent race).
  • Tests: subprocess tests set env on the child Command; in-process tests use the temp-env crate (scoped + serialized). Result: no unsafe and no #[allow(unsafe_code)] anywhere; deny stays meaningful.

Dependencies

  • Hoist serde, serde_json, tokio (union of features), futures, tar, sha2, regex, zip, tempfile, assert_cmd, testcontainers into [workspace.dependencies]. Fixes the confirmed regex (1.0 vs 1.10) and zip (2.1 vs 2.4) divergences.

Packaging / build profiles

  • publish = false on both crates (this is a consumer CLI shipped as prebuilt binaries, not a crates.io library); documentation now points at the repo.
  • [profile.release]: lto = "thin", codegen-units = 1, strip = true — and drop the fragile shell strip … || true from the release workflow (rustc strips cross-platform now). Moved dev debug = 1 from a global rustflag (which also bloated release) into [profile.dev].

CI / hygiene

  • clippy now runs with --all-features (previously feature-gated full code was unlinted in CI).
  • Standardize codeql.yml checkout to actions/checkout@v6.
  • Clarify the full = [] compile-gate feature comment.
  • Silence a pre-existing dead-code warning in the --no-default-features MVP build (resolve_features_ordered is only reachable under full).

Deliberately NOT changed

  • deny.toml multiple-versions stays warn: the duplicates are transitive (thiserror 1/2, getrandom, windows-sys, …), not hoistable — flipping to deny would break CI.
  • No crates.io publish step or [package.metadata.docs.rs] (moot under publish = false).

Verification

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • make test-nextest-fast → 2161 passed ✅
  • default / --no-default-features MVP / --release builds all green ✅

🤖 Generated with Claude Code

…I, deps, profiles)

Conformance pass against Cargo Book / API-guidelines / packaging norms.

Edition & MSRV:
- Migrate both crates to edition 2024 + resolver "3" (MSRV-aware), bump
  rust-version to 1.85, run `cargo fix --edition` (match-ergonomics, import
  ordering, formatting). Add an MSRV CI job pinned to 1.85 so the declared
  rust-version is actually verified.

Edition 2024 made std::env::set_var/remove_var `unsafe`; relax the workspace
lint from `unsafe_code = "forbid"` to `deny` (still errors on any unguarded
unsafe) and remove all unsafe instead of allowing it:
- Production: cli.rs threads the default log directive into the logging
  initializer (new init_with_redaction_and_directive) instead of mutating
  RUST_LOG; docker_retry.rs replaces its env round-trip test seam with an
  atomic counter.
- Tests: subprocess tests set env on the child Command; in-process tests use
  the temp-env crate (scoped, serialized) — no unsafe, no #[allow(unsafe_code)].

Dependencies:
- Hoist serde, serde_json, tokio (union features), futures, tar, sha2, regex,
  zip, tempfile, assert_cmd, testcontainers into [workspace.dependencies];
  fixes the regex (1.0 vs 1.10) and zip (2.1 vs 2.4) divergences.

Packaging / build:
- publish = false on both crates (consumer CLI, not a crates.io library);
  point `documentation` at the repo.
- Add [profile.release] lto="thin"/codegen-units=1/strip=true and drop the
  fragile shell `strip || true` from the release workflow. Move dev `debug=1`
  from a global rustflag to [profile.dev] so it no longer bloats release.

CI / hygiene:
- clippy now runs with --all-features (was missing feature-gated code).
- Standardize codeql checkout to actions/checkout@v6.
- Clarify the `full = []` compile-gate feature comment.
- Silence a pre-existing dead-code warning in the --no-default-features MVP
  build (resolve_features_ordered is only reachable under `full`).

deny.toml multiple-versions left at "warn": the duplicates are transitive
(thiserror 1/2, getrandom, windows-sys, …), not hoistable, so flipping to
"deny" would break CI.

Full workspace gate green: fmt, clippy --all-targets --all-features -D warnings,
2161 tests, release build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added ci CI/CD changes build Build system changes deps Dependency updates labels May 29, 2026
@pofallon pofallon merged commit 98c15b7 into main May 30, 2026
11 checks passed
@pofallon pofallon deleted the chore/rust-best-practices branch May 30, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system changes ci CI/CD changes deps Dependency updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant