decrating: absorb shipper-git into shipper::ops::git#75
Merged
Conversation
Final microcrate absorption in Phase 2 — brings the workspace to the
13-crate target. The `shipper-git` microcrate (2095 LOC) and the in-tree
`crates/shipper/src/git.rs` shim (158 LOC) are collapsed into
`crates/shipper/src/ops/git/` as crate-private modules:
- `mod.rs` — public-to-crate facade + `collect_git_context`
- `cleanliness.rs` — canonical `is_git_clean` / `ensure_git_clean`
(with CLI-compatible "not clean; commit/stash changes or use
--allow-dirty" phrasing) plus the `SHIPPER_GIT_BIN` override routing
- `context.rs` — commit/branch/tag/changed-files/remote queries plus
the `get_git_context` aggregator and all 170+ unit, snapshot, and
property tests
- `bin_override.rs` — parallel `SHIPPER_GIT_BIN`-aware helpers that
preserve the in-tree shim's env-var override behavior
`GitContext` already lived in `shipper-types`; its `new`, `has_commit`,
`is_dirty`, and `short_commit` methods are moved there (matching the
pre-absorption `shipper-git` implementation byte-for-byte, including
the 7-byte short-commit slice for ASCII hex).
Backward compatibility:
- `shipper::git::*` public surface unchanged — the new `src/git.rs`
re-exports `crate::ops::git::{collect_git_context, ensure_git_clean,
is_git_clean}`.
- `SHIPPER_GIT_BIN` env override preserved.
- All 36 original snapshot files renamed and ported verbatim under
`src/ops/git/snapshots/` so the test matrix continues to pin
GitContext serialization, error phrasing, and porcelain parsing.
Also updates:
- Workspace `Cargo.toml` (drops `crates/shipper-git` member)
- `crates/shipper/Cargo.toml` (drops `shipper-git` dep)
- `fuzz/Cargo.toml` + `fuzz/fuzz_targets/git_context.rs` (switch to
`shipper_types::GitContext`)
- `docs/architecture.md` (shipper-git marked absorbed; leaf-crate list
trimmed; `micro-git` mention removed)
- `crates/shipper/src/ops/mod.rs` (registers `pub(crate) mod git`)
- `Cargo.lock` (drops shipper-git entry)
Per `docs/decrating-plan.md` §3.2. Completes Phase 2.
|
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 20 minutes and 56 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 (37)
📒 Files selected for processing (16)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request absorbs the shipper-git microcrate into the main shipper crate as the shipper::ops::git module. It relocates the GitContext struct to shipper-types and updates the workspace to reflect the removal of the standalone crate. A review comment suggests using anyhow::Context for improved error chaining in the git cleanliness check.
| return bin_override::local_is_git_clean(repo_root, &git_program); | ||
| } | ||
|
|
||
| is_git_clean_default(repo_root).map_err(|err| anyhow::anyhow!("git status failed: {err}")) |
There was a problem hiding this comment.
The error message construction here uses anyhow::anyhow!("git status failed: {err}"). While this is consistent with the existing code, it is generally better to use context from anyhow to chain errors properly, which preserves the error source and provides better debugging information.
Suggested change
| is_git_clean_default(repo_root).map_err(|err| anyhow::anyhow!("git status failed: {err}")) | |
| is_git_clean_default(repo_root).context("git status failed") |
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
Final microcrate absorption for decrating Phase 2. Collapses the standalone
shipper-gitcrate (2095 LOC) plus the in-treecrates/shipper/src/git.rsshim (158 LOC) into
crates/shipper/src/ops/git/as crate-private modules,bringing the workspace to the 13-crate target.
Layout
Backward compatibility
shipper::git::*surface unchanged.src/git.rsis now a thinre-export of
crate::ops::git::{collect_git_context, ensure_git_clean, is_git_clean}.SHIPPER_GIT_BINenv override preserved (routes throughbin_override.rs).GitContextstays inshipper-types; itsnew/has_commit/is_dirty/short_commitmethods are moved there (byte-for-byte port from the oldshipper-git crate, including 7-byte short-commit slice).
Changes
crates/shipper/src/ops/{mod.rs,git/}— new modulecrates/shipper/src/git.rs— now a facade (9 LOC, was 158)crates/shipper/Cargo.toml— dropsshipper-gitdepcrates/shipper-types/src/lib.rs— addsGitContextaccessor methodscrates/shipper-git/— deleted (crate + snapshots)Cargo.toml— workspace member removedCargo.lock— shipper-git entry droppedfuzz/Cargo.toml,fuzz/fuzz_targets/git_context.rs— switches import toshipper_types::GitContextdocs/architecture.md— shipper-git marked absorbedTest plan
cargo check --workspace— cleancargo check --workspace --all-targets— cleancargo test -p shipper ops::git— 142/142 passingcargo test -p shipper— 1830 lib tests passingcargo test -p shipper-cli— only pre-existing flakypreflight_allow_dirty_snapshotfails (verified identical failure onmain; network/mock-server timing, unrelated to this change)cargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo fmt --all -- --check— cleangrep -l shipper-git crates/*/Cargo.toml fuzz/Cargo.toml— emptyPer
docs/decrating-plan.md§3.2. Completes Phase 2.