-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add conceptual documentation for dependency mirrors #9540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add conceptual documentation for dependency mirrors #9540
Conversation
heckj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Vidit,
Thank you for taking a stab at this, we really appreciate it!
I've left a number of structural notes for you in the rest of the review - the content that you're adding it great, but I think it would be a whole lot better if it went further, and embraced a structure that matched the rest of the documentation here. I'll didn't want to assume you knew any of the odd details of DocC in particular, so I tried to lay out the feedback in a lot of detail, maybe too much - hopefully it helps.
Definitely take the time to see how it looks in context, and I think some of the feedback will make a lot more sense. You can see a preview of the content by using the command:
swift package --disable-sandbox preview-documentation --target PackageManagerDocs
This command uses the swift-docc-plugin to build and preview the content and show it to you locally. The dependency that provides it is available when you've enabled SWIFTCI_USE_LOCAL_DEPS, so if you don't have a full Swift toolchain checked, that might be awkward to use as it sits. If don't have SwiftPM checked out as part of a full toolchain setup, then it may be easier to just temporarily add the dependency locally for your own use:
.package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.1.0"),
I hope you're game to revise and expand on this content, as I think it's great to have for SwiftPM, and a wonderful start! Please feel free to reach out to me if you have questions, either through feedback here in Github, or you can find me in the unofficial open source Swift Slack channel or the Swift Forums.
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
|
Thank you for the detailed and thoughtful review — I really appreciate you taking the time to go through this. I agree with the points you raised around structure and alignment with the rest of the documentation. I’ll revise the article to refine the abstract so it better matches the DocC style used elsewhere, restructure the content to align more closely with existing conceptual documentation, and expand on the practical aspects by clarifying when and how dependency mirrors are typically configured. I’ll also add the appropriate curation so the article appears in the correct place within the documentation hierarchy. I’ll preview the changes locally using the DocC plugin before pushing an updated version. Thanks again for the guidance — it’s very helpful. |
|
I’ve updated the article to fold usage and scope details into the overview, adjusted section structure to follow DocC conventions, and expanded the configuration section to clearly describe how dependency mirrors are used in practice, including the relevant SwiftPM commands and when they take effect during dependency resolution. I’ve also removed reliance on Swift Evolution links and curated the article under the existing dependency documentation. Please let me know if you’d like the usage flow expanded further or split into a separate article. |
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Show resolved
Hide resolved
|
I’ve updated the Dependency Mirrors documentation to reflect the actual key-based mirror model used by SwiftPM. The text now clarifies that mirror configuration is indexed by the original dependency identity, that get-mirror is a lookup requiring --original, and that there’s no global enumeration of mirrors. It also documents the authoritative behavior and the implicit handling of configuration scope, all aligned with the current SwiftPM implementation. |
|
@heckj I’ve pushed the requested changes addressing the feedback above. Whenever you get a chance, I’d appreciate a quick look. Thanks! |
heckj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content looks good to me. I went back and noticed that other articles tended to have imperative phrases for the second headers, so I've made suggestions to switch those to keep this aligned.
There's also quite a lot of passive voice in the paragraphs, so I've made suggestions to work them into active voice. Feel free to adjust if you prefer a different phrasing, but we want to keep the writing voice to active voice, 2nd person imperative (using "You" instead of "I" or "We").
I'd also like to get a sign off from one of the SwiftPM engineering team folks. I believe it's all accurate as you've got it though.
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
Sources/PackageManagerDocs/Documentation.docc/Package/DependencyMirrors.md
Outdated
Show resolved
Hide resolved
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
…cyMirrors.md Co-authored-by: Joseph Heck <[email protected]>
|
@heckj Thanks for the review! I’ve incorporated your suggestions to align the headers with imperative phrasing and revised the text to use active voice and second-person language. Happy to make any further tweaks if needed, and I’ll wait for sign-off from the SwiftPM team. |
|
Hi @bripeticca , I noticed you’ve reviewed similar docs changes before, so tagging you here. |
This PR adds a conceptual documentation page explaining Swift Package Manager dependency mirrors.
It describes:
It also links to SE-0219 for full design details.
Fixes #8867