ChuckerTeam / chucker

🔎 An HTTP inspector for Android & OkHTTP (like Charles but on device)
Apache License 2.0
3.93k stars 344 forks source link

One variant for Chucker instead of two (library & library-no-op) #752

Open noureldeen-abouelkassem opened 2 years ago

noureldeen-abouelkassem commented 2 years ago

:warning: Is your feature request related to a problem? Please describe

It is a request to make chucker has only one variant and can be defined in one line. Two lines/variants for Chucker is really a headache if your project is modularized. Also if you use chucker with a library you will have to make the library has the same variants as chucker to avoid class/resource duplications.

:raising_hand: Do you want to develop this feature yourself?

MiSikora commented 2 years ago

So, my answer depends on what is meant by a single variant. If it is about bundling two libraries into one artifact, I'm against that. The issue was discussed some time ago in #362, and my position did not change.

However, if it is about dropping the no-op variant I wouldn't automatically dismiss it, and it comes with pros and cons.

Pros:

Cons:

IMO the first con heavily outweighs any pros. Especially because pros are mostly on our side, not the consumer side. In the current setup, it is still possible to not use the no-op variant, and no one is forced to add it. That's what I do in my projects.

Libraries that use Chucker internally are expected to provide no-op variants of their own. Alternatively, they should provide some form of DI. Whether it is OkHttpClient, Interceptor, or ChuckerInterceptor it's up to the authors.

cortinico commented 2 years ago

It is a request to make chucker has only one variant and can be defined in one line.

Just as a side note: nothing is preventing you from using just a implementation(...:library:+) dependency without using the library-no-op artifact at all.

You would then have to handle some configuration manually (e.g. adding the Interceptor only on Debug build, making sure that the notification is not shown, making sure that the Chucker Activity is not exported), but it's totally doable.

Two lines/variants for Chucker is really a headache if your project is modularized. Also if you use chucker with a library you will have to make the library has the same variants as chucker to avoid class/resource duplications.

Agree that having a library in between complicates everything due to variant matching. What would be the preferred approach for you? Also how is your intermediate library handling the release vs debug setup?

vbuberen commented 2 years ago

I would say that I am against having just one artifact for both debug and production builds. The main reason is safety. Even with 2 artifacts it is not guaranteed that library users will ship the correct one, like happened with LeakCanary some time ago: https://twitter.com/Piwai/status/1245524534712602624?s=20

But I am also open to hear other suggestions on this topic.