If what we are doing is just making sure that each geode module is explicitly declaring all of its first order dependencies (things that directly show up in import statements) in its build.gradle, then this seems good to me.
I don't think we want to stop using gradle's ability to pull in all of the transitive implementation dependencies that we don't directly use. We don't want to get into the business of maintaining those manually. Removing unused dependencies and pulling in this lint plugin also seem like good improvements. -Dan On Mon, Oct 1, 2018 at 1:44 PM Patrick Rhomberg <prhomb...@apache.org> wrote: > > Patrick, you just mentioned the java-library plugin's ability to > > distinguish between api and implementation dependencies. Geode doesn't > use > > that plugin yet AFAIK, nor does the PR introduce that plugin right? > > That is correct, we do not yet make use of the `java-library` plugin (which > has the `implementation` and `api` configurations) and are still using the > `java` plugin (and rely on the `compile` configurations). I brought it up > only to mention that there will be a great deal of inter-module build > breakages if we were to try to transition to using `implementation` while > our actual dependencies of each module were not yet explicit. > > And yeah, all of this is on the road to Java9 modules. > > As for accepting this PR, there are still multiple issues with it as it > stands -- I mostly opened it in its current state to spur this discussion. > I'm was going to go open a handful of tickets so that each individual PR > and commit are more bite-sized. > > On Fri, Sep 28, 2018 at 4:26 PM, Bill Burcham <bburc...@pivotal.io> wrote: > > > So it sounds like (per Robert) we use the gradle-lint-plugin's > > unused-dependency rule to warn us of unused dependencies. That handles > one > > side of the equation (only list *necessary* dependencies). > > > > I haven't heard anyone mention tool support for the other side of the > > equation: ensuring that we list the *sufficient* list of dependencies per > > build.gradle. > > > > Patrick, you just mentioned the java-library plugin's ability to > > distinguish between api and implementation dependencies. Geode doesn't > use > > that plugin yet AFAIK, nor does the PR introduce that plugin right? > > > > All this seems of a piece with the march toward modularity, exemplified > by > > Java 9 modules. Patrick, is your vision something like: > > > > 1. accept this PR > > … > > 2. introduce the java-library plugin and segregate dependencies into api > > vs. implementation ones > > … > > N. define Java modules > > > > > > On Fri, Sep 28, 2018 at 10:39 AM Patrick Rhomberg <prhomb...@apache.org> > > wrote: > > > > > Bill has the heart of it, yes. > > > > > > I should have also mentioned that this ties into java-library plugin > > > configuration, notably that the `compile` configuration is deprecated. > > > For dependences that we do not wish to leak, we will need to use > > > `implementation`. For dependencies which we intentionally elect to > share > > > with our consumers, we should use `api`. > > > > > > For modularity, it should not break one module's ability to build for > > > another module to declare a `compile` configuration to be > > `implementation`, > > > unless of course that dependency is an active and necessary component > > part > > > of that module's consumption. In that case, it should be declared part > > of > > > the modules `api`. > > > In this vein and as Bill pointed out, a module should have an accurate > > > representation of its own build-time dependencies. > > > > > > In time, this will allow a much improved build experience, since > > > `implementation` and `api` are positioned for better granularity of > > > incremental building. > > > > > > On Fri, Sep 28, 2018 at 10:23 AM, Robert Houghton < > rhough...@pivotal.io> > > > wrote: > > > > > > > Hi Bill, > > > > > > > > We are using a Gradle plug-in to identify dependencies that are > unused, > > > or > > > > are declared in the wrong module or scope. > > > > This is called out by the Gradle documents on improving build [ > > > > https://guides.gradle.org/performance/#avoid_unnecessary_and_unused_ > > > > dependencies]. > > > > The plug-in documentation is here [ > > > > https://github.com/nebula-plugins/gradle-lint-plugin/ > > > > wiki/Unused-Dependency-Rule > > > > ] > > > > > > > > Thank you, > > > > -Robert Houghton > > > > > > > > On Fri, Sep 28, 2018 at 10:18 AM Bill Burcham <bburc...@pivotal.io> > > > wrote: > > > > > > > > > From the PR, Anthony, it seems to me that Patrick is proposing that > > > each > > > > > build.gradle be explicit about mentioning the "things" it depends > on. > > > For > > > > > example: > > > > > > > > > > [image: image.png] > > > > > > > > > > Look at how geode-connectors goes from mentioning a few > dependencies > > to > > > > > mentioning many more. The value of this is that it ostensibly shows > > us > > > > all > > > > > the things that geode-connectors actually depends on. > > > > > > > > > > The challenge I see is two-fold: > > > > > > > > > > • as with Java imports and "unused imports", there is the risk of > > > listing > > > > > more dependencies than we actually need > > > > > • there's also the risk that we still don't have a complete list of > > > > > dependencies > > > > > > > > > > Patrick: do you have tool support for this? Is there a tool that > can > > > > > identify or even remove unused dependencies? What process do you > use > > > for > > > > > finding these heretofore hidden dependencies? Did you run gradle > > > > > app:dependencies? > > > > > > > > > > > > > > >
