-
Notifications
You must be signed in to change notification settings - Fork 143
removes logic duplicated in SymbolKit #1091
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
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.
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
- prefer SymbolKit for this logic
9fa5bab
to
df2fb86
Compare
@swift-ci test |
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.