docs(specification): centralize shared types per reference.md (#412)#443
Open
sriakula1 wants to merge 1 commit into
Open
docs(specification): centralize shared types per reference.md (#412)#443sriakula1 wants to merge 1 commit into
sriakula1 wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…sal-Commerce-Protocol#412) PR Universal-Commerce-Protocol#226 made `reference.md` the canonical render destination for all `types/` schemas (cross-references already resolve there via the redirect logic in `main.py`). However, capability pages still re-rendered the same `types/...` field tables inline, leaving three copies of each shared type (cart.md, checkout.md, catalog/index.md) alongside the canonical render in reference.md. This change finishes the centralization: * `schema_fields(entity, spec)` and `extension_schema_fields(entity, spec)` now detect when the requested schema lives under a `types/` directory (common or vertical) and, for any caller other than `reference.md`, emit a Markdown link to the canonical entry instead of an inline table. * `reference.md` continues to render the full tables via `auto_generate_schema_reference`, so the source of truth is unchanged. * Existing capability-page macro calls (`{{ schema_fields('types/ signals', 'cart') }}`, etc.) work unmodified; the surrounding prose context on each capability page is preserved. The macro change is intentionally minimal: no `.md` files are touched, keeping the diff narrow and reversible. Out of scope (follow-ups): * wry-ry's suggestion in the issue thread to replace the "Required" column with a "Visibility" column (`required` / `optional` / `omit`) is deferred — it needs an `omit` annotation in the schemas which doesn't exist today. Worth a separate issue once the data model is agreed.
f8d385f to
3ff2d30
Compare
Author
|
@googlebot I signed it! |
1 similar comment
Author
|
@googlebot I signed it! |
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
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.
Description
Closes #412.
PR #226 made
reference.mdthe canonical render destination for alltypes/schemas — cross-references already resolve there via the redirect increate_link. The gap @igrigorik called out: capability pages still re-render the sametypes/...field tables inline, so each shared type (Buyer, Signals, Attribution, Total, Line Item, Message, Link, …) is rendered three times — once each incart.md,checkout.md,catalog/index.md— alongside the canonical render inreference.md.From the issue, the desired end state:
This PR implements that without churning a single
.mdfile.What changed
main.pyonly. Two macros now detect when the requested schema lives under atypes/directory (common/types/or any<vertical>/types/) and, when the caller is anything other thanreference.md, emit a Markdown link to the canonical entry instead of rendering the table inline:schema_fields(entity, spec)— handles calls likeschema_fields('types/signals', 'cart'),schema_fields('buyer', 'checkout'),schema_fields('types/line_item_create_req', 'cart'), etc. Renders:extension_schema_fields(entity, spec)— handles calls likeextension_schema_fields('types/pagination.json#/$defs/response', 'cart')andextension_schema_fields('types/payment_instrument.json#/$defs/selected_payment_instrument', 'checkout'). Renders a single anchor link.reference.mdstill renders the full tables (viaauto_generate_schema_referenceand direct macro calls withspec_file_name='reference'); the source of truth is unchanged.All existing capability-page macro calls (
{{ schema_fields('types/signals', 'cart') }}, etc.) work unmodified — no.mdedits required. The surrounding prose context already present on each capability page (e.g. cart.md's paragraphs above Signals/Attribution/Total) is preserved untouched.Tradeoff acknowledged
As @igrigorik noted in the issue: cross-linking out to
reference.mddoes break the flow of reading a single capability page end-to-end. The counter-argument — that UCP has many shared types and replicating them across every capability page creates anchor-resolution problems and maintenance churn — is the reason #226 went this way, and is the same reason here.Files changed
main.py— added_resolves_to_shared_type()and_render_shared_type_link()helpers; one early-return branch inschema_fields; one early-return branch inextension_schema_fields.Local build
mkdocs buildexercises the modified macros and they short-circuit cleanly for everytypes/...capability-page caller. The full build still needs theucp-schemaRust CLI for the remaining (non-types) macro calls onreference.mditself — that's unchanged frommain, and CI runs the canonical build.Verified via a small harness (mock mkdocs env, call the macro directly):
schema_fields('types/signals', 'cart')→ link toreference.md#signals✓schema_fields('buyer', 'checkout')→ link toreference.md#buyer✓schema_fields('types/line_item_create_req', 'cart')→ link toreference.md#line-item-create-request✓extension_schema_fields('types/pagination.json#/$defs/response', 'cart')→reference.md#pagination-response✓schema_fields('types/signals', 'reference')→ falls through to inline render (correct) ✓Deferred (worth a separate issue)
@wry-ry suggested replacing the "Required" column with a "Visibility" column (
required/optional/omit) on rendered tables. That's a great direction but needs an explicitomitannotation in the schemas, which isn't in the data model today. I left it out of this PR so the diff stays narrow and reviewable — happy to follow up with a separate proposal once the schema-side question is settled.Type of change
Reviewer notes
Tagging @igrigorik and @wry-ry since you both engaged on the issue. Per
MAINTAINERS.md,docs/specification/ultimately wants tech-council eyes — please route as appropriate.