decrating: Phase 6 — schema vs types audit [FOLD]#77
Merged
Conversation
Audit found shipper-schema was two string-parsing functions (parse_schema_version, validate_schema_version), ~57 lines of implementation, one anyhow dep, and zero external consumers. The format it parses (`shipper.<doctype>.vN`) is hard-coded into the function itself — no conceptual independence from shipper's state-file types. - Move src/lib.rs to crates/shipper-types/src/schema.rs (with inline tests) - Move external integration tests to crates/shipper-types/tests/ - Rename snapshots to match new module path (shipper_types__schema__tests__*) - Add anyhow dep to shipper-types - Update shipper_schema::X -> shipper_types::schema::X across: shipper-config, shipper (state::store, state::execution_state), fuzz target - Remove shipper-schema from workspace members, Cargo.toml dep lists, mutation/release CI workflows, and RELEASE_CHECKLIST_v0.3.0.md - Delete crates/shipper-schema/ - Update docs/decrating-plan.md Phase 6 with decision rationale - Update docs/architecture.md tables and dependency graph Drops the published-crate count from 13 to 12. All shipper-types tests pass (245 unit + 17 schema_snapshot + 2 schema_contract). Full workspace cargo check and cargo fmt clean.
There was a problem hiding this comment.
Code Review
This pull request implements Phase 6 of the decrating effort by folding the standalone shipper-schema crate into shipper-types as a dedicated schema module. The changes involve moving the core logic and snapshot tests, updating workspace dependencies in Cargo.toml and Cargo.lock, and performing a mechanical rename of call sites across the shipper-config and shipper crates. Documentation, including the architecture guide and decrating plan, has been updated to reflect this consolidation. I have no feedback to provide.
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
Phase 6 of the decrating effort: audit whether
shipper-schemashould remain a standalone published crate or be folded intoshipper-types.Decision: FOLD.
shipper-schemais nowshipper_types::schema. The published-crate count drops from 13 to 12.Audit findings
parse_schema_version,validate_schema_versionsrc/lib.rs(rest was tests)anyhowonlyu32/Result<()>shipper-config,shipper(state::store, state::execution_state),fuzz, testsshipper.<doctype>.vNformat is hard-coded into the parse function itselfAgainst the rubric in the task spec, every "fold" criterion was met and no "keep" criterion applied.
What changed
crates/shipper-schema/src/lib.rs→crates/shipper-types/src/schema.rs(withpub mod schema;inshipper-types/src/lib.rs)crates/shipper-types/tests/schema_contract_integration.rsandschema_snapshot_tests.rsshipper_schema__tests__*→shipper_types__schema__tests__*;snapshot_tests__*→schema_snapshot_tests__*) with theirsource:paths updatedanyhow = "1.0"toshipper-types(same depshipper-configandshipperalready use)shipper_schema::X→shipper_types::schema::Xrename across five filesshipper-schemafrom: workspace members,shipper/Cargo.toml,shipper-config/Cargo.toml,fuzz/Cargo.toml.github/workflows/mutation.yml,.github/workflows/release.yml,docs/decrating-plan.md(Phase 6 section + ring 1 table + publish order),docs/architecture.md(tables + dependency graph),docs/testing.md,RELEASE_CHECKLIST_v0.3.0.mdcrates/shipper-schema/directoryTest plan
cargo check --workspace— cleancargo test -p shipper-types— 245 unit + 17 schema_snapshot + 2 schema_contract + 3 doc tests all pass (snapshots matched exactly after rename + source-path rewrite)cargo test -p shipper-config— 15 tests passcargo test -p shipper --lib state::store::tests— schema-related tests passcargo test -p shipper --lib state::execution_state::tests::parse_schema_version— 7 tests passcargo clippy -p shipper-types --all-targets --all-features -- -D warnings— cleancargo fmt --all -- --check— clean