decrating: absorb shipper-process into shipper::ops::process#55
Conversation
|
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 35 minutes and 13 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 (16)
📒 Files selected for processing (23)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16ffc58b87
ℹ️ 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".
| micro-registry = ["shipper/micro-registry"] | ||
| micro-process = ["shipper/micro-process"] | ||
| micro-policy = ["shipper/micro-policy"] |
There was a problem hiding this comment.
Preserve
micro-process feature alias for shipper-cli
Removing the micro-process feature from shipper-cli makes the existing CI matrix fail because .github/workflows/ci.yml still runs cargo test -p shipper-cli --features "micro-process" (BDD job, line 213/233), which now errors with the package 'shipper-cli' does not contain this feature. This blocks that matrix leg (and any downstream automation still using the feature flag), so the feature removal needs to be coordinated with workflow updates or kept as a compatibility alias in the same change.
Useful? React with 👍 / 👎.
Sweep-cleanup of CI workflows, templates, and docs for references to microcrates and micro-* feature flags that have been deleted or are queued for deletion via absorption PRs #52 #54 #55 #56 #57 #58. Changes: - .github/workflows/ci.yml - removed deleted features (micro-lock, micro-plan, micro-policy, micro-process, micro-store) from BDD matrix - .github/workflows/mutation.yml - removed shipper-plan, shipper-policy, shipper-levels from the mutation-testing target list - .github/workflows/release.yml - added note that publish order is finalized in Phase 8 of the decrating plan - templates/circleci-config.yml - matching feature-matrix cleanup - docs/architecture.md - per-crate absorbed notes in microcrates table plus top-of-section notes on Dependency Graph and Module Responsibilities pointing out those sections reflect pre-decrating state - docs/testing.md - removed deleted crates from example test invocations and the mutation-testing example - RELEASE_CHECKLIST_v0.3.0.md - header note listing absorbed crates and pruned publish steps - RELEASE_NOTES_v0.3.0.md - reworded Modular Architecture bullet to reflect the consolidated public-crate layout References to still-in-flight absorptions (auth, environment, git, storage, engine-parallel, registry, progress) are left alone - they will be cleaned up in their respective absorption PRs. Per docs/decrating-plan.md.
Move shipper-process crate (1948 LOC) into crates/shipper/src/ops/process/ as crate-private modules, split into types.rs, run.rs, timeout.rs, which.rs, cargo.rs + mod.rs facade, tests.rs, snapshot_tests.rs, and cross_platform_edge_case_tests.rs. Delete the stale 105-LOC duplicate at crates/shipper/src/process.rs and the 32-LOC shim at process_micro.rs. Remove shipper-process from workspace members and from shipper's deps. Drop the micro-process feature flag (from both shipper and shipper-cli Cargo.toml). Add `which = "8.0"` as a shipper dep and enable `yaml` feature on insta dev-dep to support the co-located snapshot tests. Per docs/decrating-plan.md §6 Phase 2.
16ffc58 to
01e813b
Compare
Sweep-cleanup of CI workflows, templates, and docs for references to microcrates and micro-* feature flags that have been deleted or are queued for deletion via absorption PRs #52 #54 #55 #56 #57 #58. Changes: - .github/workflows/ci.yml - removed deleted features (micro-lock, micro-plan, micro-policy, micro-process, micro-store) from BDD matrix - .github/workflows/mutation.yml - removed shipper-plan, shipper-policy, shipper-levels from the mutation-testing target list - .github/workflows/release.yml - added note that publish order is finalized in Phase 8 of the decrating plan - templates/circleci-config.yml - matching feature-matrix cleanup - docs/architecture.md - per-crate absorbed notes in microcrates table plus top-of-section notes on Dependency Graph and Module Responsibilities pointing out those sections reflect pre-decrating state - docs/testing.md - removed deleted crates from example test invocations and the mutation-testing example - RELEASE_CHECKLIST_v0.3.0.md - header note listing absorbed crates and pruned publish steps - RELEASE_NOTES_v0.3.0.md - reworded Modular Architecture bullet to reflect the consolidated public-crate layout References to still-in-flight absorptions (auth, environment, git, storage, engine-parallel, registry, progress) are left alone - they will be cleaned up in their respective absorption PRs. Per docs/decrating-plan.md.
Summary
shipper-processmicrocrate (1948 LOC) intocrates/shipper/src/ops/process/as crate-private modules. Seedocs/decrating-plan.md§6 Phase 2.types.rs(result types),run.rs(basic runners),timeout.rs(timeout-aware runner),which.rs(PATH lookups),cargo.rs(cargo convenience wrappers), plus three test modules (tests.rs,snapshot_tests.rs,cross_platform_edge_case_tests.rs). A thinmod.rsfacade re-exports what the single in-tree caller (crate::cargo) needs.crates/shipper/src/process.rsand the 32-LOC shim atcrates/shipper/src/process_micro.rs. Dropsshipper-processfrom workspace members and fromshipper's dependencies, plus themicro-processfeature in bothshipperandshipper-cli. Addswhich = "8.0"toshipperand enables theyamlfeature on theinstadev-dep so the co-located snapshot tests keep working.Visibility
All items in
ops::processarepub(crate)— this subsystem is an internal implementation detail.crate::cargo(the only call site) goes throughcrate::ops::process::run_command_with_timeout.Conflict notes
crates/shipper/src/ops/mod.rs— this PR appendspub(crate) mod process;. The concurrent lock-absorption PR also modifies this file to appendpub(crate) mod lock;. Expect a small, trivially resolvable textual conflict when the second PR lands.crates/shipper/Cargo.toml,crates/shipper-cli/Cargo.toml, and the rootCargo.tomlworkspace members list are edited by several concurrent absorption PRs at disjoint lines — git auto-merge should handle those.Test plan
cargo check --workspace --all-targets— clean.cargo clippy --workspace --all-targets --all-features -- -D warnings— clean.cargo fmt --all -- --check— clean.cargo test -p shipper --lib ops::process— 113 tests pass (matches the original microcrate's test count).cargo test -p shipper --lib— 881 tests pass.cargo test -p shipper-cli— 10 tests pass.shipper_process__snapshot_tests__*toshipper__ops__process__snapshot_tests__*withsource:paths updated;instaaccepts them without regeneration.