docs: document the local: bool parameter across bake and resolution APIs#69
Open
bkfunk wants to merge 1 commit into
Open
docs: document the local: bool parameter across bake and resolution APIs#69bkfunk wants to merge 1 commit into
bkfunk wants to merge 1 commit into
Conversation
The `local: bool` flag threads through `LibraryLoader::load_ref`, `ResolutionResult::query`, and `bake_part_from_*`, but its meaning was nowhere stated. Callers reading from one site couldn't tell whether the flag had the same semantics at the next site, and the bake-time flag silently has to match the resolution-time flag. Adds doc comments at the four sites that take `local: bool` explaining that it distinguishes user-supplied / working-directory parts (`true`) from stock library parts (`false`), and noting that the bake-time value must match the resolution-time value. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves API documentation by clearly defining the semantics of the local: bool flag across library loading, resolution lookup, and part baking entry points, including the requirement that the bake-time and resolution-time local values must match to avoid incorrect sub-part resolution.
Changes:
- Documented
local: boolsemantics onLibraryLoader::load_ref. - Documented lookup behavior and the meaning of the returned
boolonResolutionResult::query. - Documented
local: boolsemantics (and mismatch hazard) forbake_part_from_multipart_documentand referenced it frombake_part_from_document.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
ldraw/src/library.rs |
Adds rustdoc clarifying local resolution/search behavior for load_ref and ResolutionResult::query (including meaning of the returned bool). |
ir/src/part.rs |
Adds rustdoc explaining how local affects sub-part resolution during baking and that it must match the value used during dependency resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
The
local: boolflag threads through several entry points but has no doc comment anywhere:LibraryLoader::load_refResolutionResult::querybake_part_from_multipart_documentbake_part_from_documentA reader at any one of those sites can't tell whether the flag has consistent semantics across them, or that the bake-time value silently has to match the resolution-time value. Callers (including in-tree tools and downstream consumers) tend to pass `false` by convention rather than understanding.
Change
Adds doc comments at the four sites explaining:
No code changes — only doc comments.
Test plan