-
Notifications
You must be signed in to change notification settings - Fork 131
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
removes logic duplicated in SymbolKit #1091
base: main
Are you sure you want to change the base?
Conversation
92fc12e
to
74059be
Compare
@swift-ci please test |
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.
LGTM. Waiting for the symbolkit PR and other's suggestion before a merge.
- prefer SymbolKit for this logic
74059be
to
9fa5bab
Compare
(rebased, but otherwise no changes in that last commit) |
@swift-ci test |
|
||
let moduleName: String | ||
if isMainSymbolGraph || symbolGraph.module.bystanders != nil { |
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.
Hmm, I see that this version has a symbolGraph.module.bystanders != nil
check, but the SymbolKit version doesn't. Do we need it? cc @QuietMisdreavus
While debugging #1084, I found this duplicate logic (and the source of the issue). There's an open pull request to resolve the error in SymbolKit (swiftlang/swift-docc-symbolkit#89), and this compliments that pull request to resolve #1084, using the logic in SymbolKit over duplicate, local logic.
I opted for SymbolKit's implementation in this case because its method is public, while the local method was private.