Support for package-private jsonAdapter method

Question

Support for package-private jsonAdapter method

nedtwigg opened this issue 8 years ago · 7 comments

auto-value-moshi requires a method like this:

public static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... }

Are you open to a PR which would allow this?

static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... }

With Java 9 modularity coming, I'd like the option to use Moshi as an implementation detail which is invisible to client code.

Answer 1 · 2017-03-11T12:06:24.000Z

That would require all your model objects be in the same package. Is that ok? Compilation would fail otherwise, which seems like reasonable behavior.

…

On Sat, Mar 11, 2017, 3:03 AM Ned Twigg ***@***.***> wrote: auto-value-moshi requires a method like this: public static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } Are you open to a PR which would allow this? static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } With Java 9 modularity coming, I'd like the option to use Moshi as an implementation detail which is invisible to client code. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEVEDY6C-hZIvAg7pEG5YZGz-Cv6Xks5rklVugaJpZM4MaI1N> .

Answer 2 · 2017-03-11T13:13:50.000Z

That's only the case if you're using the MoshiAdapterFactory, right? If accessing the adapters manually or another way then they could be in different packages.

…

On Sat, Mar 11, 2017, 6:06 AM Jake Wharton ***@***.***> wrote: That would require all your model objects be in the same package. Is that ok? Compilation would fail otherwise, which seems like reasonable behavior. On Sat, Mar 11, 2017, 3:03 AM Ned Twigg ***@***.***> wrote: > auto-value-moshi requires a method like this: > > public static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } > > Are you open to a PR which would allow this? > > static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } > > With Java 9 modularity coming, I'd like the option to use Moshi as an > implementation detail which is invisible to client code. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#87>, or mute the > thread > < https://github.com/notifications/unsubscribe-auth/AAEEEVEDY6C-hZIvAg7pEG5YZGz-Cv6Xks5rklVugaJpZM4MaI1N > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPJbi1rqZSVmpFaHp3EQYeNa2y2rnNFks5rko5AgaJpZM4MaI1N> .

Answer 3 · 2017-03-11T13:44:34.000Z

Yeah. I was thinking about models that referenced other models in other packages specifically. I still think it's reasonable to allow.

…

On Sat, Mar 11, 2017, 8:13 AM Ryan Harter ***@***.***> wrote: That's only the case if you're using the MoshiAdapterFactory, right? If accessing the adapters manually or another way then they could be in different packages. On Sat, Mar 11, 2017, 6:06 AM Jake Wharton ***@***.***> wrote: > That would require all your model objects be in the same package. Is that > ok? Compilation would fail otherwise, which seems like reasonable behavior. > > On Sat, Mar 11, 2017, 3:03 AM Ned Twigg ***@***.***> wrote: > > > auto-value-moshi requires a method like this: > > > > public static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { > ... } > > > > Are you open to a PR which would allow this? > > > > static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } > > > > With Java 9 modularity coming, I'd like the option to use Moshi as an > > implementation detail which is invisible to client code. > > > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub > > <#87>, or mute the > > thread > > < > https://github.com/notifications/unsubscribe-auth/AAEEEVEDY6C-hZIvAg7pEG5YZGz-Cv6Xks5rklVugaJpZM4MaI1N > > > > . > > > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > < #87 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABPJbi1rqZSVmpFaHp3EQYeNa2y2rnNFks5rko5AgaJpZM4MaI1N > > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEZsd3MI6WUgbeHGMo5KvWMldZqsyks5rkp4OgaJpZM4MaI1N> .

Answer 4 · 2017-03-11T13:48:43.000Z

I agree, seems like a simple enough option that doesn't hinder any current functionality. On Sat, Mar 11, 2017 at 7:44 AM Jake Wharton <notifications@github.com> wrote:

…

Yeah. I was thinking about models that referenced other models in other packages specifically. I still think it's reasonable to allow. On Sat, Mar 11, 2017, 8:13 AM Ryan Harter ***@***.***> wrote: > That's only the case if you're using the MoshiAdapterFactory, right? If > accessing the adapters manually or another way then they could be in > different packages. > > On Sat, Mar 11, 2017, 6:06 AM Jake Wharton ***@***.***> > wrote: > > > That would require all your model objects be in the same package. Is that > > ok? Compilation would fail otherwise, which seems like reasonable > behavior. > > > > On Sat, Mar 11, 2017, 3:03 AM Ned Twigg ***@***.***> > wrote: > > > > > auto-value-moshi requires a method like this: > > > > > > public static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { > > ... } > > > > > > Are you open to a PR which would allow this? > > > > > > static JsonAdapter<MyAutoValueClass> jsonAdapter(Moshi moshi) { ... } > > > > > > With Java 9 modularity coming, I'd like the option to use Moshi as an > > > implementation detail which is invisible to client code. > > > > > > — > > > You are receiving this because you are subscribed to this thread. > > > Reply to this email directly, view it on GitHub > > > <#87>, or mute the > > > thread > > > < > > > https://github.com/notifications/unsubscribe-auth/AAEEEVEDY6C-hZIvAg7pEG5YZGz-Cv6Xks5rklVugaJpZM4MaI1N > > > > > > . > > > > > > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub > > < > #87 (comment) > >, > > or mute the thread > > < > https://github.com/notifications/unsubscribe-auth/ABPJbi1rqZSVmpFaHp3EQYeNa2y2rnNFks5rko5AgaJpZM4MaI1N > > > > . > > > > — > You are receiving this because you commented. > > > Reply to this email directly, view it on GitHub > < #87 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAEEEZsd3MI6WUgbeHGMo5KvWMldZqsyks5rkp4OgaJpZM4MaI1N > > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPJblDg-hLDQgDL_zKF6zPhqgFeIw4eks5rkqVDgaJpZM4MaI1N> .

Answer 5 · 2019-01-13T11:15:10.000Z

Eyeballed nedtwigg@cd905ac

I think that mostly works, but we should do extra checks at compile-time to basically make sure we're only including models from the same package if we do this, otherwise you could get confusing error messages about not generating anything if you have multiple factories in different packages

Answer 6 · 2019-07-02T04:14:07.000Z

Looks like there's been quite a bit of really useful stuff since 0.4.5: 0.4.5...master

In addition to this issue, upgrading to AutoValue 1.6 and implementing net.ltgt.gradle.incap.IncrementalAnnotationProcessorType.AGGREGATING would be big wins. I know there's always more I try to stuff into my releases, but it would be really cool to get an 0.4.6 as-is out on MavenCentral. It's 0.x stuff, no warranty expected :)

Answer 7 · 2019-08-05T04:31:01.000Z

WAAAHOOOO!!! Thanks so much for 0.4.6! This is such a useful library, great to see it shipping again :)