Ibotta / gradle-aspectj-pipeline-plugin

A Gradle plugin for Android projects which performs AspectJ weaving using Android's bytcode manipulation pipeline.
Apache License 2.0
75 stars 15 forks source link

A few questions #1

Closed jdvp closed 3 years ago

jdvp commented 3 years ago

Hey @eschlenz, I discovered this plugin through your article while actually trying fix a problem with another plugin (lol) by getting away from said plugin, since I was trying to fix my own AOP Sample Application bc new views on one of my older articles would probably expect it to be working properly.

First off, this is very cool and switching plugins was super easy from my end, and worked without any issues. Great work!

My questions are as follows:

  1. What's stopping this library itself from becoming unmaintained? I think you may be the only contributor for now, which may not necessarily be a bad thing but what if you leave Ibotta or something? Would someone there still actively maintain it? I'm sure if AOP is a big part of your process, they would be forced to but you know.
  2. Does the AGP version matter at all for this plugin? You mentioned the bytecode pipeline, so I was curious. For example, in my previous experience, Gradle and AGP updates frequently broke the other plugin as APIs changed. Do you think it will be an issue for this plugin? If not, that's actually a huge benefit over existing solutions.

Thanks again for the cool plugin! Please feel free to close this issue as you see fit as it's not actually a bug or anything.

jdvp commented 3 years ago

Or also before I forget, the plugin directive: com.ibotta:plugin is a little vague and may cause issues if you want more ibotta plugins later. I'm not too familiar with plugins but you may want to consider renaming it at some point com.ibotta:aspectj-plugin

eschlenz commented 3 years ago

Hey there @jdvp,

Happy to hear you were able to use our plugin successfully! In response to your questions:

  1. At this time, I personally am the one who did the majority of this work on this plugin, and pushed to open source it. That being said, we have a pretty good sized Android team who could help maintain this in the future. Would that happen if I left? I'd hope so, but I honestly can't promise anything. FWIW, I've been at Ibotta for over 8 years, and don't have plans to leave any time soon. :) We are also open to ideas on how to promote the maintainability of this plugin moving forward.
  2. I think the AGP updates could potentially break things, but I'm not particularly concerned about the bytecode pipeline mechanisms changing much. Again, I can't promise anything. It looks like the mechanism was introduced about three years ago, and the function's contract hasn't changed since then. That doesn't mean the implementation hasn't changed, but based on what I know, it doesn't seem like a feature that is under heavy modification.

Or also before I forget, the plugin directive: com.ibotta:plugin is a little vague and may cause issues if you want more ibotta plugins later. I'm not too familiar with plugins but you may want to consider renaming it at some point com.ibotta:aspectj-plugin

Guhh, I couldn't agree more. I'll try to explain what happened here.

We have our plugin being distributed to the "Gradle Plugin Portal", which seems appropriate. In order to do so, you use another Gradle plugin to publish it.

The variables you specify, and what happens with them, was not obvious nor was the documentation particularly helpful. We thought we had it configured with sane values, but had no way to really confirm that without actually releasing the plugin. So we tried that. Then we found out that there are huge delays in getting through the Gradle Plugin Portal approval process (could be weeks). Once we finally got approved after tapping a contact we have at Gradle, we came to find out the plugin identifier was less than desirable.

Honestly, the huge delay in approvals, and the need to figure out the right way to configure the plugin's group identifier, are what made us leave it as is for now. We (Ibotta) need to devote some time to this, unless someone from the community can help out sooner. Wink wink, nudge nudge. If for some reason it causes huge headaches, other than just being unsightly and ambiguous, I could try to push internally for some dedicated time to fix it. As with any open source project though, we are hoping to get some help from the community whenever possible.

jdvp commented 3 years ago

@eschlenz Thanks for the quick reply. This all makes a lot of sense to me, so I'm going to update my article's example and project with the Ibotta plugin as a replacement for what I had before.

The bytecode pipeline is pretty interesting so I'll also take a look into it, I never even knew it existed. But I guess that's true for a lot of things that are hidden away in gradle and android builds!

I'm not very familiar with working on plugins themselves but I'll definitely play with this one and maybe one day I'll be able to contribute!

If for some reason it causes huge headaches, other than just being unsightly and ambiguous, I could try to push internally for some dedicated time to fix it.

I don't think this a serious issue, unless it's causing problems for you guys. It is ambiguous but I don't think it will cause any major issues for anyone.

Thanks again and have a nice holiday!

eschlenz commented 3 years ago

The bytecode pipeline is pretty interesting so I'll also take a look into it, I never even knew it existed. But I guess that's true for a lot of things that are hidden away in gradle and android builds!

It is! We hadn't heard of it either until our Gradle reps pointed us in that direction. In hindsight, it makes sense that such a feature exists.