Skip to content

Conversation

@NightlyNexus
Copy link
Contributor

This is handy for development builds where kapt is disabled and models still have @JsonClass(generatedAdapter = true).

Closes #715

@swankjesse
Copy link
Collaborator

Not sure about this. Though it’s convenient for development it also hides configuration bugs.

@swankjesse
Copy link
Collaborator

Also needs a test to somehow confirm it’s doing the right thing.

What’d you think about enabling this behavior via some Moshi configuration? Perhaps this:

Moshi moshi = new Moshi.Builder()
    .failOnMissingCodegen(false)
    .build()

Or perhaps we just recover gracefully if the reflection adapter is installed:

Moshi moshi = new Moshi.Builder()
    .add(new KotlinJsonAdapterFactory())
    .build()

@NightlyNexus
Copy link
Contributor Author

Definitely don't want to hide configuration errors. My thought was that development builds would turn off kapt and provide a Moshi instance with the KotlinJsonAdaterFactory is installed, and production builds would turn on kapt and not install the KotlinJsonAdapterFactory. I don't know how common it is to use both in production.

perhaps we just recover gracefully if the reflection adapter is installed

If I understand correctly, that is the behavior of this change?

Copy link
Collaborator

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

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

Ahhh, yes I get it. Can you add a test? You’ll need to put it in the reflection adapter’s module so it doesn’t get codegen

This is handy for development builds where kapt is disabled and models still have @JsonClass(generatedAdapter = true).
@NightlyNexus NightlyNexus force-pushed the eric.reflection-for-fallback branch from 53b17c2 to 032d877 Compare November 4, 2018 23:00
@swankjesse swankjesse merged commit 5c41565 into master Nov 6, 2018
@NightlyNexus NightlyNexus deleted the eric.reflection-for-fallback branch November 6, 2018 07:42
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