Skip to content
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

Remove @_implementationOnly annotations #616

Merged

Conversation

keith
Copy link
Contributor

@keith keith commented Feb 15, 2024

These annotations produce warnings when compiling swift-syntax without library evolution using Swift ≥5.10.

Replace them by private import when compiling using Swift ≥5.11.

Mirrors swiftlang/swift-syntax#2429

Checklist

@keith
Copy link
Contributor Author

keith commented Feb 15, 2024

@swift-ci please test

@keith keith force-pushed the ks/remove-_implementationonly-annotations branch from 5736e59 to 6432e19 Compare February 16, 2024 18:43
@keith
Copy link
Contributor Author

keith commented Feb 16, 2024

@swift-ci please test

@keith
Copy link
Contributor Author

keith commented Feb 16, 2024

@natecook1000 can you review? (sorry I can't add you as a reviewer here)

@rauhul
Copy link
Contributor

rauhul commented Feb 16, 2024

Is there a reason to use private instead of internal?

Additionally could we use an upcoming feature flag instead of conditional compilation?

@keith
Copy link
Contributor Author

keith commented Feb 16, 2024

I don't have a pref on private vs internal for this case, was just trying to mirror similarly to what was here before. But I guess if private works I'd think we should prefer that, wdyt? Based on swiftlang/swift-syntax#2429 (comment) you cannot use that flag instead

@rauhul
Copy link
Contributor

rauhul commented Feb 16, 2024

I wasn't sure if there was a specific reason to prefer private other than minimally scoped imports. Given the context of the swift-syntax comment, this changes lgtm, but I'll wait for Nate to do the final approval

These annotations produce warnings when compiling swift-syntax without library evolution using Swift ≥5.10.

Replace them by `private import` when compiling using Swift ≥5.11.

Mirrors swiftlang/swift-syntax#2429
@keith keith force-pushed the ks/remove-_implementationonly-annotations branch from 6432e19 to 51e348b Compare February 23, 2024 18:21
@keith
Copy link
Contributor Author

keith commented Feb 23, 2024

@swift-ci please test

@natecook1000 natecook1000 merged commit c413809 into apple:main Feb 24, 2024
2 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants