To the contrary, this POM has the potential of causing more conflicts given the unnecessary declaration of dependencies. I believe if committing early and often, but when it is warranted, such as with proper testing. Committing for the sake of committing is does not fuel an OS project.
On Sun, May 17, 2015 at 2:18 PM, Roman Shaposhnik <[email protected]> wrote: > On Fri, May 15, 2015 at 5:48 PM, John Blum <[email protected]> wrote: > > I have a verdict... the Apache Geode "generated" Maven POM files need a > lot > > of work yet. > > Sure. And thanks for looking into this. My question, however, is this: it > seems > to me that the current patch proposed on GEODE-23 makes a tiny step in > the right direction. Sure it doesn't address some of the issues you're > referring > to, but it makes life just a tiny little bit better. So why not commit it > while > still working on a more complete solution? > > Thanks, > Roman. > > > First off, trying to review the geode_dependencies.txt file Mark attached > > was, well, cumbersome and tedious, prone to error. I'd rather us > generate > > appropriate looking POM files and use Mavens tools to inspect the > dependency > > tree (i.e. mvn dependency:tree) for each of Geode's POM files based on > the > > modules, similar to what Andy Wilkinson did in his Spring Boot PR review > > (https://github.com/spring-projects/spring-boot/issues/2884). > > > > Second, it is confusing after running a complete build of Apache Geode, > that > > I seem to have duplicate POM files.... > > > > $ find . -name "*.pom" > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224636-1.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224913-2.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-jgroups/1.0.0-incubating-SNAPSHOT/gemfire-jgroups-1.0.0-incubating-20150513.224637-1.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-jgroups/1.0.0-incubating-SNAPSHOT/gemfire-jgroups-1.0.0-incubating-20150513.224914-2.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-joptsimple/1.0.0-incubating-SNAPSHOT/gemfire-joptsimple-1.0.0-incubating-20150513.224637-1.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-joptsimple/1.0.0-incubating-SNAPSHOT/gemfire-joptsimple-1.0.0-incubating-20150513.224914-2.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-json/1.0.0-incubating-SNAPSHOT/gemfire-json-1.0.0-incubating-20150513.224637-1.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-json/1.0.0-incubating-SNAPSHOT/gemfire-json-1.0.0-incubating-20150513.224914-2.pom > > ... > > .. > > . > > > > With seemingly no difference between them, e.g. ... > > > > $ diff > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224636-1.pom > > > ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224913-2.pom > > $ > > > > Anyway, we have several "chores" to align the Geode and GemFire POMs more > > closely, including, but not limited to the following... > > > > 1. The <repositories> declarations need to be cleaned up (reduced). > > > > 2. Add proper "scopes", "optional" settings and "exclusions" to the > > dependencies declared in the POM. > > > > 3. Many of the "explicitly" declared dependencies (e.g. spring-beans) > need > > to be removed; Maven will resolve dependencies transitively. > > > > There may be additional tasks we need to look at here; this is on first > > glance. > > > > I have attached *new* GemFire 8.2 Maven POM file as a reference for what > the > > Geode POM files should emulate, specifically at the "intersection" of > their > > dependencies. Note, I realize there are going to be (expected) > differences > > between GemFire and Geode, such as, but not limited to, removing XOM, > while > > adding others. > > > > Again, the GemFire 8.2 POM should serve as a model template on which to > > basis the new Geode "generated" POM file. > > > > Mark and I will need to work together to make this happen. This is not > only > > important, but crucial for projects like Spring Data GemFire, Spring > Boot, > > and (potentially) many others that will develop applications for Apache > > Geode. > > > > So let's get it right! > > > > Thanks, > > John > > > > > > > > On Fri, May 15, 2015 at 3:23 PM, Anthony Baker <[email protected]> > wrote: > >> > >> I believe John is pointing out that the Geode POM may have similar > issues > >> as were discovered by GemFire users (after all, Geode was recently born > from > >> GemFire). > >> > >> However I believe those issues are limited to certain use cases (e.g. > >> spring boot). In GEODE-23 the POM is unusable without adding exclude > >> clauses. IMHO, we should fix this now and have a separate discussion > around > >> making library versions consistent. > >> > >> Anthony > >> > >> > >> > On May 15, 2015, at 11:24 AM, Roman Shaposhnik <[email protected]> > >> > wrote: > >> > > >> > Part of the urgency is that we now have nightly artifacts > >> > that are not easily consumable. > >> > > >> > On top of that, I see no reason to hold up anything in > >> > the Apache project because of a commercial product > >> > requirements. The way you explained your refactoring > >> > sounds like it benefits a commercial product. Am I getting > >> > this wrong? > >> > > >> > Thanks, > >> > Roman. > >> > > >> > On Fri, May 15, 2015 at 11:08 AM, John Blum <[email protected]> wrote: > >> >> I am in the process of refactoring GemFire's (8.0, 8.1 and 8.2) Maven > >> >> POM > >> >> files now (along with juggling 5 other things). > >> >> > >> >> What is this exactly holding up (what's the urgency)? I don't see an > >> >> issue > >> >> with delaying this PR until it is properly reviewed. An improper POM > >> >> file > >> >> does cause issues for "consumers". > >> >> > >> >> I should be able to get to this by EOD. > >> >> > >> >> Thanks! > >> >> John > >> >> > >> >> > >> >> On Fri, May 15, 2015 at 10:30 AM, Roman Shaposhnik > >> >> <[email protected]> > >> >> wrote: > >> >> > >> >>> Big +1 to that. Commit early commit often. We can always > >> >>> change it later if needed too. > >> >>> > >> >>> Thanks, > >> >>> Roman. > >> >>> > >> >>> On Fri, May 15, 2015 at 8:01 AM, Anthony Baker <[email protected]> > >> >>> wrote: > >> >>>> John, have you had a chance to review Mark’s change and the > generated > >> >>> POM? I think that we could address updates to library versions > >> >>> through a > >> >>> follow up issue. > >> >>>> > >> >>>> Anthony > >> >>>> > >> >>>>> On May 13, 2015, at 10:22 AM, John Blum <[email protected]> wrote: > >> >>>>> > >> >>>>> Mark, > >> >>>>> > >> >>>>> I plan on working on cleaning up the GemFire Maven POM today for > >> >>>>> 8.0.0, > >> >>>>> 8.1.0 and 8.2.0, and we need to make sure that Apache Geode's > Maven > >> >>>>> POM > >> >>>>> matches or is similar to clean up the other issues that *Andy > >> >>>>> Wilkinson* > >> >>>>> <https://github.com/spring-projects/spring-boot/issues/2884> [0] > >> >>>>> sited. > >> >>>>> > >> >>>>> Please hold off on the commit until I get the chance to review. > >> >>>>> > >> >>>>> Thanks, > >> >>>>> John > >> >>>>> > >> >>>>> > >> >>>>> On Wed, May 13, 2015 at 10:17 AM, Mark Bretl <[email protected]> > >> >>>>> wrote: > >> >>>>> > >> >>>>>> Hi All, > >> >>>>>> > >> >>>>>> I have tried to follow the email threads on the review process, > not > >> >>> exactly > >> >>>>>> sure what to do in this case. I am requesting review of GEODE-23 > >> >>>>>> and my > >> >>>>>> patch is attached to the JIRA. It is a one line change which > >> >>>>>> removes > >> >>> Spring > >> >>>>>> Data GemFire from the Maven POM. > >> >>>>>> > >> >>>>>> I would like to use a 72 hour lazy consensus. > >> >>>>>> > >> >>>>>> Best Regards, > >> >>>>>> > >> >>>>>> -- > >> >>>>>> Mark Bretl > >> >>>>>> Software Build Engineer > >> >>>>>> Pivotal > >> >>>>>> 503-533-3869 > >> >>>>>> www.pivotal.io > >> >>>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> -- > >> >>>>> -John > >> >>>>> 503-504-8657 > >> >>>>> john.blum10101 (skype) > >> >>>> > >> >>> > >> >> > >> >> > >> >> > >> >> -- > >> >> -John > >> >> 503-504-8657 > >> >> john.blum10101 (skype) > >> > > > > > > > > -- > > -John > > 503-504-8657 > > john.blum10101 (skype) > -- -John 503-504-8657 john.blum10101 (skype)
