Skip to content

CLI hygiene: pub→pub(crate) + RUST_LOG integration + DRY test helpers #140

@r6e

Description

@r6e

Source

Adversarial Phase 1 code review (S5, S6, S7). Bundled — all CLI/test polish, all small.

S5: Extract duplicated wire-format test helpers

write_fstring/write_pak_entry/in-data-header-size triple is duplicated across at least three test compilation units:

  • crates/paksmith-core/tests/pak_integration.rs:95,106
  • tests/fixtures/generate.rs:32,41,56
  • crates/paksmith-core/src/container/pak/index/mod.rs:578,597,609 (in-source tests)

testing::v10::write_fstring is already in __test_utils proving the pattern works; the same lift would consolidate the v3-v9 helpers. CLAUDE.md: "DRY on second occurrence — extract immediately when repetition appears."

Fix: promote write_fstring, write_pak_entry, and the wire-size helper into paksmith_core::testing::wire (or testing::v3_v9), gated on __test_utils. Replace the three duplications.

S6: CLI binary's pub items should be pub(crate)

Workspace has missing_docs = "warn". CLI uses pub (not pub(crate)) on OutputFormat, ResolvedFormat, print_entries, Command, run, ListArgs, commands::list, etc. Two issues:

  1. pub is meaningless for a [[bin]] crate, so the visibility lies about exposure intent
  2. Every one triggers missing_docs = "warn" against the workspace lint

Fix: tighten to pub(crate) (kills the missing-doc warnings simultaneously, makes the binary's internal surface honest). Files: crates/paksmith-cli/src/commands/mod.rs:1,8,14, crates/paksmith-cli/src/commands/list.rs:12,21, crates/paksmith-cli/src/output.rs:10,17,33,47.

S7: --verbose ignores RUST_LOG env var

crates/paksmith-cli/src/main.rs:32-49tracing_subscriber::fmt::try_init() is called with EnvFilter::new("debug") or "warn", both hardcoded. If the user sets RUST_LOG=paksmith_core::container::pak=trace, it's ignored because EnvFilter::new(_) does not consult RUST_LOG (that needs try_from_default_env() or from_default_env()).

Fix: chain EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(if cli.verbose { "debug" } else { "warn" })) so RUST_LOG overrides the default. Files: crates/paksmith-cli/src/main.rs:32-49.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorCode restructuring without behavior change

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions