feat(packaging): unify shipper CLI into the shipper crate (#95)#135
Merged
Conversation
Reframed packaging UX. Supersedes the parked three-crate facade (#123). ## What changes - **\`shipper\`** becomes a single public crate with both \`[lib]\` and \`[[bin]]\`. The CLI lives at \`shipper::cli\` (module, pub) and the binary target at \`src/bin/shipper.rs\` is a three-line shim over \`shipper::cli::run()\`. - **\`shipper-cli\`** stays published as a compatibility shim: \`[[bin]] name = \"shipper-cli\"\` that forwards into \`shipper::cli::run()\`. Operators on the old name keep working; new setups should \`cargo install shipper --locked\`. - README, docs, release workflow updated to prefer \`cargo install shipper\` everywhere. ## Why this shape - **One public product story.** Library users \`shipper = \"0.3\"\`, operators \`cargo install shipper\`. One crate to learn, document, cite. - **No dep cycles.** The earlier three-crate \`shipper-core\` + \`shipper\` + \`shipper-cli\` facade kept tripping cycle risk because the facade depended on the CLI which depended on the library. A single crate with a \`cli\` submodule sidesteps this. - **No hidden core.** \`shipper-core\` as a publicly meaningful crate would have forced consumers to choose between \`shipper\` and \`shipper-core\` for no gain. - **Smaller churn than the three-crate attempt.** One directory move (\`shipper-cli/src/\` → \`shipper/src/cli/\`) + bin rename, vs the ~570-file shuffle the earlier attempt produced. ## Mechanical changes - \`shipper-cli/src/main.rs\` + \`shipper-cli/src/output/\` moved under \`shipper/src/cli/\` (git mv preserves history). \`fn main\` became \`pub fn run\`. All \`use shipper::...\` rewrote to \`use crate::...\`. - \`shipper/Cargo.toml\` gains \`[[bin]] name = \"shipper\"\` + the CLI's deps (clap, clap_complete, indicatif). - \`shipper-cli/Cargo.toml\` reduces to a shim: \`[[bin]] name = \"shipper-cli\"\`, \`src/main.rs\` is three lines. - \`shipper-cli/tests/\` preserved (25 files, extensive coverage). Every \`cargo_bin!(\"shipper\")\` → \`cargo_bin!(\"shipper-cli\")\`. Tests now exercise the shim, which internally calls the same \`shipper::cli::run\` — so functional coverage is identical. - Help-text snapshots regenerated: \`Usage: shipper-cli\` when invoked via the shim (correct behavior — it IS shipper-cli when you run it that way). \`cli::output::progress\` snapshots moved to their new module path under \`shipper/src/cli/\`. - Workspace \`default-members\` reordered (\`shipper\` first). - Release workflow builds \`-p shipper\` instead of \`-p shipper-cli\`, installs \`target/release/shipper\` to PATH (facade bin name). - Docs (README, release-runbook, run-in-github-actions, tutorials, failure-modes, preflight, tech) all switch to \`cargo install shipper --locked\`. ## Acceptance - [x] \`cargo install --path crates/shipper --locked\` installs a working \`shipper\` binary. - [x] \`cargo install --path crates/shipper-cli --locked\` still works (produces a \`shipper-cli\` binary via the shim). - [x] \`shipper --version\` and \`shipper-cli --version\` both print the version. - [x] Library imports (\`use shipper::engine;\` etc.) unchanged. - [x] \`cargo clippy --workspace --all-targets --all-features -- -D warnings\` clean. - [x] \`cargo doc -p shipper --no-deps\` clean. - [x] \`cargo fmt --all --check\` clean.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 52 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (82)
📒 Files selected for processing (49)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Apr 18, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf73a687bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # `shipper`'s own `shipper` binary. | ||
| [[bin]] | ||
| name = "shipper" | ||
| name = "shipper-cli" |
There was a problem hiding this comment.
Preserve
shipper binary name in compatibility crate
Renaming the shim binary to shipper-cli breaks the advertised migration path for users who still run cargo install shipper-cli and then invoke shipper: Cargo installs executables using [[bin]].name, so those workflows now fail with shipper: command not found after upgrading this crate. If this crate is meant to be a compatibility shim, it needs to continue providing a shipper executable (or an equivalent alias) during the deprecation window.
Useful? React with 👍 / 👎.
Fixes CI failures on Ubuntu + macOS where the bin has no .exe suffix. The normalize_output / normalize_stderr helpers were stripping `shipper.exe` but not `shipper-cli.exe`, so my Windows-regenerated snapshots leaked the platform-specific suffix. Order matters in the rewrite: strip `shipper-cli.exe` → `shipper-cli` first, else the subsequent `shipper.exe` → `shipper` rule would eat the `-cli` suffix after trimming `.exe`.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Closed
5 tasks
EffortlessSteven
added a commit
that referenced
this pull request
Apr 18, 2026
Reworking #95 into the three-crate split the user specified originally: shipper (installable facade) -> shipper-cli (real CLI adapter) [PR 2] -> shipper-core (pure library/engine) [THIS PR] This PR is the bottom layer only. It moves every non-CLI module out of `crates/shipper/src/` into a new `crates/shipper-core/` crate and leaves `shipper` as a thin re-export facade so the rest of the workspace (shipper-cli, integration tests, downstream code) keeps compiling unchanged. ## Why this reopens #95 #135 merged a different shape: `shipper` carried the lib + bin, with `shipper-cli` as a 9-line compat shim. That conflated engine code and CLI code in one crate, left `shipper-cli` as a shim with no value, and didn't match the design goal of a no-clap library surface. The three-crate split below is what was originally asked for. ## What moved All moved via `git mv` so history is preserved: - `crates/shipper/src/{engine,state,ops,plan,runtime}/` → shipper-core - `crates/shipper/src/{config,encryption,git,types,webhook, property_tests,stress_tests}.rs` → shipper-core - `crates/shipper/src/lib.rs` → shipper-core (then edited to drop the `pub mod cli;` declaration and retarget docs at `shipper_core`) - proptest-regressions fixtures moved with their owning tests What stayed in `crates/shipper/`: - `src/cli/` (moves to `shipper-cli` in PR 2) - `src/bin/shipper.rs` (stays — this crate owns the installable binary) ## Facade The new `crates/shipper/src/lib.rs` re-exports shipper-core's public surface (auth, cargo, cargo_failure, config, encryption, engine, git, lock, plan, registry, retry, runtime, state, store, types, webhook) so every existing `shipper::X` import path still resolves. PR 3 slims this to a curated re-export list once `shipper-cli` takes over the CLI. ## Snapshots 326 insta snapshots renamed `shipper__*.snap` → `shipper_core__*.snap` to match the new crate prefix in `module_path!()`. Tests pass with the renamed files; no content changes. ## CI Architecture-guard workflow updated to watch `crates/shipper-core/src/**` (where the layer dirs now live) in addition to `crates/shipper/src/**`, so the upward-import check keeps enforcing instead of silently skipping. ## Verification - `cargo check --workspace --all-targets` — clean - `cargo clippy --workspace --all-targets --all-features -- -D warnings` — clean - `cargo test -p shipper-core --lib` — 1889 passed - `cargo test -p shipper` — 39 passed - Full workspace test run — all passed except pre-existing Windows- specific flakes (`concurrent_version_exists_checks` on macOS, `preflight_command_*` temp-dir locking on Windows); both pass in isolation and are unrelated to this split. ## What this PR does not do - Does not move the CLI (that's PR 2) - Does not slim `crates/shipper`'s Cargo.toml or README (PR 3) - Does not touch workspace-wide docs or how-to guides (PR 4)
6 tasks
EffortlessSteven
added a commit
that referenced
this pull request
Apr 18, 2026
* feat(packaging): create shipper-core library crate (#95 PR 1) Reworking #95 into the three-crate split the user specified originally: shipper (installable facade) -> shipper-cli (real CLI adapter) [PR 2] -> shipper-core (pure library/engine) [THIS PR] This PR is the bottom layer only. It moves every non-CLI module out of `crates/shipper/src/` into a new `crates/shipper-core/` crate and leaves `shipper` as a thin re-export facade so the rest of the workspace (shipper-cli, integration tests, downstream code) keeps compiling unchanged. ## Why this reopens #95 #135 merged a different shape: `shipper` carried the lib + bin, with `shipper-cli` as a 9-line compat shim. That conflated engine code and CLI code in one crate, left `shipper-cli` as a shim with no value, and didn't match the design goal of a no-clap library surface. The three-crate split below is what was originally asked for. ## What moved All moved via `git mv` so history is preserved: - `crates/shipper/src/{engine,state,ops,plan,runtime}/` → shipper-core - `crates/shipper/src/{config,encryption,git,types,webhook, property_tests,stress_tests}.rs` → shipper-core - `crates/shipper/src/lib.rs` → shipper-core (then edited to drop the `pub mod cli;` declaration and retarget docs at `shipper_core`) - proptest-regressions fixtures moved with their owning tests What stayed in `crates/shipper/`: - `src/cli/` (moves to `shipper-cli` in PR 2) - `src/bin/shipper.rs` (stays — this crate owns the installable binary) ## Facade The new `crates/shipper/src/lib.rs` re-exports shipper-core's public surface (auth, cargo, cargo_failure, config, encryption, engine, git, lock, plan, registry, retry, runtime, state, store, types, webhook) so every existing `shipper::X` import path still resolves. PR 3 slims this to a curated re-export list once `shipper-cli` takes over the CLI. ## Snapshots 326 insta snapshots renamed `shipper__*.snap` → `shipper_core__*.snap` to match the new crate prefix in `module_path!()`. Tests pass with the renamed files; no content changes. ## CI Architecture-guard workflow updated to watch `crates/shipper-core/src/**` (where the layer dirs now live) in addition to `crates/shipper/src/**`, so the upward-import check keeps enforcing instead of silently skipping. ## Verification - `cargo check --workspace --all-targets` — clean - `cargo clippy --workspace --all-targets --all-features -- -D warnings` — clean - `cargo test -p shipper-core --lib` — 1889 passed - `cargo test -p shipper` — 39 passed - Full workspace test run — all passed except pre-existing Windows- specific flakes (`concurrent_version_exists_checks` on macOS, `preflight_command_*` temp-dir locking on Windows); both pass in isolation and are unrelated to this split. ## What this PR does not do - Does not move the CLI (that's PR 2) - Does not slim `crates/shipper`'s Cargo.toml or README (PR 3) - Does not touch workspace-wide docs or how-to guides (PR 4) * fix(shipper-core): update lock doctest imports to shipper_core Three doctests in `ops/lock/mod.rs` still used `use shipper::lock::LockFile;` after the #95 PR 1 split. Doctests compile as external code against the crate being tested, so the crate name now has to be `shipper_core`. Caught by the `cargo test --workspace --doc` lane in CI (not covered by `cargo test -p shipper-core --lib` locally).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The reframed #95 landing. Supersedes the parked three-crate facade
experiment on #123.
`shipper` becomes a single public crate carrying both its library API
and the `shipper` binary. `shipper-cli` stays published as a
compatibility shim. `cargo install shipper --locked` is now the paved
road.
Shape
CLI lives in `shipper::cli` (module, pub). Binary target at
`src/bin/shipper.rs` is a three-line shim calling `shipper::cli::run()`.
(renamed to avoid workspace collision), `src/main.rs` is three lines,
forwards to `shipper::cli::run()`. Deprecation window — operators
still referencing it get a working CLI during migration.
Why not `shipper-core`
Captured the reasoning on #95 + #123 earlier. Short version:
pick between `shipper` and `shipper-core` for no gain.
Mechanical churn
`crates/shipper/src/cli/{mod.rs,output}` (history preserved).
and the `[[bin]]` target.
Every `cargo_bin!("shipper")` → `cargo_bin!("shipper-cli")` so tests
exercise the shim (which internally dispatches to the same
`shipper::cli::run`).
to the new module path under `shipper/src/cli/`.
preflight, tech) switch to `cargo install shipper`.
Test plan
flakiness under -j N remains, passes -j 1 — not caused by this PR).
Sequencing
Per the agreed plan: trust-core → Prove → Remediate → #95 (this) → #96.
#96 (Trusted Publishing, draft at #122) follows once this lands and
can be rebased onto the final packaging layer.