-
Notifications
You must be signed in to change notification settings - Fork 304
Fix visibility on import triggering a compile warning #2151
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?
Conversation
40a62e6
to
e888c87
Compare
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.
Thanks for opening the PR to fix this. Looks good to me.
@swift-ci Please test |
Head branch was pushed to by a user without write access
e888c87
to
f44489c
Compare
Argh, the |
Thanks ❤️ |
@swift-ci Please test |
@swift-ci Please test Windows |
Looks like there are some more import-related errors. Could you take a look at them? If you can’t figure them out, let me know and I can help investigate |
Previously, building sourcekit-lsp would produce the following warning when building on macOS: $ swift build Building for debugging... /Users/wilfred/src/sourcekit-lsp/Sources/SKLogging/CustomLogStringConvertible.swift:13:9: warning: package import of 'Foundation' was not used in package declarations 11 | //===----------------------------------------------------------------------===// 12 | 13 | package import Foundation | `- warning: package import of 'Foundation' was not used in package declarations Remove the visibility modifier, so no warning is produced.
Head branch was pushed to by a user without write access
f44489c
to
6b4b4b2
Compare
OK, I've tweaked the conditional to check whether we're on macOS, which seems to fix the build. I don't understand why the warning only triggered on macOS though. I see
This is the build failure I see on my Linux box, which suggests that the Linux build is treating TLDR: This fixes the warning and I think should make CI green, although there's some detail I don't understand. |
I believe the difference here is where We could maybe make this:
To make it super obvious, or just:
? |
Previously, building sourcekit-lsp would produce the following warning:
Remove the visibility modifier, so no warning is produced.