-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add deprecation warnings for package-internal APIs #315
Conversation
Signed-off-by: Michael Rebello <[email protected]>
@@ -25,6 +25,11 @@ extension ConnectError { | |||
/// passed in the headers block for gRPC-Web. | |||
/// | |||
/// - returns: A tuple containing the gRPC status code and an optional error. | |||
@available( | |||
swift, | |||
deprecated: 100.0, |
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.
This could be misleading. Is it dangerous to use 1.1 instead? If we end up needing a minor release before we're actually ready to use Swift 6 package
visibility, could we just bump this out to 1.2? Would that cause havoc?
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.
This refers to the version of Swift that this will be deprecated in - it's just an arbitrarily high number which doesn't show up in the warning message. Unfortunately, we cannot specify which version of the package something will be deprecated in as part of this macro
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.
Oh. So why not set it to 6?
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.
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.
Per separate chat, let's also rename these to start with an underscore -- yet another convention to indicate the symbol is private and that users should avoid it. That way we should be comfortable that a future release that makes them inaccessible will not break anyone.
Otherwise, LGTM.
Signed-off-by: Michael Rebello <[email protected]>
9b677fd
to
1b02c8d
Compare
Updated to prefix with underscores 👍🏽 |
This PR adds deprecation warnings to APIs that are considered package-internal but cannot yet be marked as
package
instead ofpublic
until we adopt Swift 6 (see #310). We can't make these APIs currently deprecated since that will introduce deprecation warnings where they're used within the library, but we can at least add future deprecation warnings which will show up in the IDE as shown below:The goal of doing this is to help enable us to tag a 1.0 before adopting Swift 6 so we can mark those APIs as
package
afterwards and reasonably consider it a non-breaking change.