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

Make enum as sealed class Unknown constructor opt-in #5813

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

BoD
Copy link
Contributor

@BoD BoD commented Apr 15, 2024

Related to #5796

This uses @ApolloInternal- should we introduce a dedicated annotation instead?

@BoD BoD requested a review from martinbonnin as a code owner April 15, 2024 12:55
Copy link

netlify bot commented Apr 15, 2024

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 0ad8ee4
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/661d315bd8009c00087fb15d

@martinbonnin
Copy link
Contributor

martinbonnin commented Apr 15, 2024

should we introduce a dedicated annotation instead?

I'd vote yes here to make the intent really clear. There might be use cases where opting-in is desirable and we'll guarantee the API while opting-in @ApolloInternal is always dangerous with no API guarantee.

Maybe @ApolloEnumConstructor?

Ping @StylianosGakis any preference?

@StylianosGakis
Copy link
Contributor

I think a separate annotation makes sense yes. Exactly because it may be something that you may want to opt-in without it being the "wrong" thing to do. While using something ApolloInternal should always be the "wrong" thing to do.
This distinction would be welcome in my opinion!

BoD and others added 2 commits April 15, 2024 15:52
…raphql/apollo3/annotations/ApolloEnumConstructor.kt

Co-authored-by: Martin Bonnin <martin@mbonnin.net>
…raphql/apollo3/annotations/ApolloEnumConstructor.kt

Co-authored-by: Martin Bonnin <martin@mbonnin.net>
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.

None yet

3 participants