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

Add configuration to generate Kotlin models with internal modifier #1818

Conversation

sav007
Copy link
Contributor

@sav007 sav007 commented Dec 1, 2019

Closes #1605

@sav007 sav007 self-assigned this Dec 1, 2019
Copy link
Contributor

@tasomaniac tasomaniac left a comment

Choose a reason for hiding this comment

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

Awesome addition. 👍 Should we do a release? We have a lot of new features

@tasomaniac
Copy link
Contributor

tasomaniac commented Dec 1, 2019

I am thinking now whether if we should segregate Kotlin only configuration extensions vs others in the incubating plug-in

@martinbonnin
Copy link
Contributor

I am thinking now whether if we should segregate Kotlin only configuration extensions vs others in the incubating plug-in

At some point I was contemplating have 2 pluginIds: com.apollographql.apollo.java and com.apollographql.apollo.kotlin. Not sure if it's really worth it or if it will bring confusion...

@tasomaniac
Copy link
Contributor

2 separate plugins may mean also code duplication. I'm not sure. Somehow separation would be useful tho. Right now, it is getting confusing too. Like this internal modifier is available to Java users. They don't need it.

@martinbonnin
Copy link
Contributor

martinbonnin commented Dec 1, 2019

2 separate plugins may mean also code duplication.

Not necessarily. We can use the same jar, 2 pluginIds and reuse as much of the plugin code as we want.

  • We'll know even before Plugin.apply() what kind of source file is going to be generated => no more need for afterEvaluate to setup the sourceSets
  • Only expose in the gradle configuration the parameters that are useful for the targetted language.

Somehow separation would be useful tho. Right now, it is getting confusing too

Agreed. So something like?

apollo {
    outputPackageName.set("com.example")
    kotlinOptions {
       generateAsInternal.set(true)
    }
    javaOptions {
       nullableValueType.set("annotated")
    }
}

Like this internal modifier is available to Java users

It's caught at runtime at the moment with checks like this:
https://github.com/martinbonnin/apollo-android/blob/bea404bc589e298715c8aad3ea5d0675165b3181/apollo-gradle-plugin-incubating/src/main/kotlin/com/apollographql/apollo/gradle/internal/ApolloGenerateSourcesTask.kt#L123
Agreed compile time would be better.

@sav007 sav007 force-pushed the feature-1605/generate-kotlin-models-as-internal branch from 71316ea to 5b420d1 Compare December 2, 2019 17:12
@sav007 sav007 merged commit da1ae19 into apollographql:master Dec 2, 2019
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.

Modify generated code visibility in Kotlin
3 participants