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

@experimental -> @requiresOptIn #4175

Merged
merged 4 commits into from
Jun 13, 2022
Merged

@experimental -> @requiresOptIn #4175

merged 4 commits into from
Jun 13, 2022

Conversation

martinbonnin
Copy link
Contributor

Rename @experimental to @requiresOptIn

Also:

  • support renaming @targetName
  • add tests that read the opt-in annotations using reflection

@@ -519,16 +517,17 @@ interface Service {
val sealedClassesForEnumsMatching: ListProperty<String>

/**
* The annotation to use for experimental fields/inputFields/enumValues
* The annotation to use for `@requiresOptIn` fields/inputFields/enumValues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is all still "cooking" :) but it would make sense to have a mapping (feature -> annotation) instead?

So for instance

type Foo {
  field: String
  betaField: String @requiresOptIn(feature: "beta")
  alphaField: String @requiresOptIn(feature: "alpha")
}

could generate

data class Foo(
  field: String,
  @Beta
  betaField: String,
  @Alpha
  alphaField: String,
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good point. Ultimately, I think we'll want to do this with GraphQL configuration, not Gradle configuration:

extend directive @requiresOptIn @map(feature: "alpha", annotation: "com.example.Alpha")
                     @map(feature: "beta", annotation: "com.example.Beta")

or if we decide to go the "markers" road:

directive @alpha on FIELD @requiresOptIn
directive @beta on FIELD @requiresOptIn

extend directive @alpha @targetName(name: "com.example.Alpha")
extend directive @beta @targetName(name: "com.example.Beta")

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah doing this with GraphQL is nice! I guess it'll depend on which direction the RFC goes but both approaches look good to me :)

Base automatically changed from directive-rename to main June 7, 2022 09:50
@netlify
Copy link

netlify bot commented Jun 7, 2022

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 9c65662
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/629f8f65eeeb1600099f0cd5

@martinbonnin martinbonnin merged commit 907f7c0 into main Jun 13, 2022
@martinbonnin martinbonnin deleted the requiresOptIn branch June 13, 2022 09:06
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

2 participants