refactor!: remove duplicate public paths before 1.0 - #319
Merged
Merged
Conversation
Resolve the duplicate-path decision in #315 with its proposed default: - settings::schema is now internal; its types and functions (including the previously unexported validate_catalog) are public only from settings directly. - actions no longer re-exports ActionLayout, ActionLayoutError, or action_width; they are public only from emitter, the path already used by opy-rs. - rules no longer re-exports validate_canonical_ids; it is public only from validate. - The root, program::, and per-domain Program model re-exports (actions::{Action, ModifyOp}, events::*, rules::{Program, Rule, Condition, Subroutine, Variable}, values::Value) are kept and documented as intentional in docs/compatibility-facades.md, along with the already-documented detect and signatures aliases. Verified with cargo semver-checks that only the three intended paths are flagged as removed, cargo test --workspace (all 231+ tests pass), and cargo clippy --workspace --all-targets --all-features -D warnings clean.
- actions and rules re-export more than just the Program model types (element-count analysis types and semantic-inspection types respectively); say so instead of claiming 'only'. - ADR-0006 still pointed at settings::schema, which is now internal; point at settings and the compatibility doc instead. Found by an independent review of #319.
indent
Bot
force-pushed
the
teakowa/issue-315-duplicate-public-paths
branch
from
September 27, 2026 11:45
9663c79 to
57d003b
Compare
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
2 participants
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.
Closes #315.
Summary
Resolves the duplicate-path decision in #315 with its proposed default, so every public item is either reachable through exactly one path or documented as an intentional 1.x alias.
settings::schemais now internal (pub(crate)). Its types and functions, including the previously unexportedvalidate_catalog, are re-exported flat fromsettingsonly. Internal/external callers ofsettings::schema::{validate_catalog, definitions}were updated tosettings::{validate_catalog, definitions}(workshop-catalog-gen,workshop-rs-cli::census,tests/public_api.rs).actionsno longer re-exportsActionLayout,ActionLayoutError,action_width. They stay public only fromemitter— the path opy-rs already uses.actions::layoutis nowpub(crate)andemitterre-exports from it directly.rulesno longer re-exportsvalidate_canonical_ids. It stays public only fromvalidate(rules::validatewas alreadypub(crate), unchanged).docs/compatibility-facades.mdas intentional: the root/program::/per-domainProgrammodel re-exports (actions::{Action, ModifyOp},events::*,rules::{Program, Rule, Condition, Subroutine, Variable},values::Value), plus the already-documenteddetectandsignaturesaliases. The doc also now correctly notes thatactions/rulesre-export more than just the model types (element-count analysis, semantic inspection) rather than overclaiming "only".docs/adr/0006-settings-semantic-schema.mdthat still pointed atsettings::schemaas the public path.Why these three
settings::schema,actions::{ActionLayout, ActionLayoutError, action_width}, andrules::validate_canonical_idswere implementation-phase re-exports with no discoverability purpose beyond the domain/operation module that already owns them. The model re-exports are kept because the crate's own docs presentactions,events,rules, andvaluesas the discoverable Workshop domains alongsideprogram, so removing those would work against the crate's documented design.Verification
Rebased onto current
main(post #317/#318, now v0.10.0) after an independent review caught that the branch was stale and had a real merge conflict intests/public_api.rsagainst #317's newmapped_text_is_constructible_outside_the_cratetest; that review also caught the two doc overclaims/staleness fixed above. Re-verified after rebase:cargo test --workspace: 231 passed (+ 9/103/15/15 across the other test binaries), 0 failed, 3 ignored (pre-existing/unrelated).cargo clippy --workspace --all-targets --all-features -- -D warnings: clean.cargo semver-checks --release-type minor -p workshop-rsagainst the current v0.10.0 baseline: flags exactly the three removed paths (settings::schemamodule + its 20 types/functions,actions::{ActionLayout, ActionLayoutError, action_width},rules::validate_canonical_ids) and nothing else (4 failure categories, 192/196 checks pass).!marker per the pre-1.0 breaking-change convention.Non-goals
No renaming or reorganizing beyond removing the duplicate paths, per the issue's non-goals. Downstream import migrations (opy-rs, deltin-rs) are out of scope; a GitHub code search confirmed neither currently imports the three removed paths, but consumers own their own migrations regardless.
Tag
@indentto continue the conversation here.