You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by richardcloudsoft <gi...@git.apache.org> on 2014/07/22 13:34:24 UTC
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
GitHub user richardcloudsoft opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/85
Various dependency clean-ups
Attempts to make the POM files a more accurate representation of what each module is actually using, and to standardise on placing versions in the root POM *only* (in the `dependencyManagement` section)
Testing: full unit test run; started Brooklyn and deployed MySQL+Tomcat cluster to localhost
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/richardcloudsoft/incubator-brooklyn pom-dependency-fixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/85.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #85
----
commit a4b52ca8a558bbcc4a81cebe87d94c94935c7348
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-07T16:01:25Z
Fix "Used undeclared" from mvn dependency:analyze
Means that the dependencies actually *used* in a module's code are
reflected in pom.xml (and not merely implicitly on the classpath due to
a transitive dependency)
commit 51cd8141d26d68a26afb36829f0e3029c4eb3115
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-09T16:07:24Z
Refer to brooklyn project inter-dependencies consistently
commit c6f4969c0330ffe1e3cf6a0e4332cf9d53ea53bb
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-21T14:01:11Z
Specify dependency versions in root pom.xml ONLY
Goes through all child pom.xml files and removes version from each
dependency; specify all versions in a <dependencyManagement> section of
the root pom.xml instead. Ensures that we will not be introducing
conflicting version numbers.
commit 8914714fb1c3a50e62de088454b080771b2d9497
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-21T14:20:33Z
Remove apache-rat-plugin version in child modules
It is only required in the root POM.
commit 839ff88bdfd6c2d4f19131cb1b4a24d38c6e0e69
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-22T10:23:22Z
fixup for a4b52ca8
commit 33f1dcffea3655e12e91cc3d409c850171b66e97
Author: Richard Downer <ri...@apache.org>
Date: 2014-07-22T11:16:36Z
Prevent swagger from importing slf4j
Without this, there will be "multiple SLF4J bindings" exceptions
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15228851
--- Diff: locations/jclouds/pom.xml ---
@@ -38,6 +38,63 @@
<dependency>
<groupId>io.brooklyn</groupId>
<artifactId>brooklyn-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-groovy</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
--- End diff --
I'll refactor shortly to remove the need for groovy in this project, except as a test dependency!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15229020
--- Diff: utils/groovy/pom.xml ---
@@ -40,13 +40,26 @@
<dependency>
<groupId>io.brooklyn</groupId>
<artifactId>brooklyn-utils-test-support</artifactId>
- <version>${brooklyn.version}</version>
+ <version>${project.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
--- End diff --
Needs scope=test presumably?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15228973
--- Diff: storage/hazelcast/pom.xml ---
@@ -48,8 +63,13 @@
<!-- test dependencies -->
<dependency>
- <groupId>${project.groupId}</groupId>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
--- End diff --
Needs scope=test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15532016
--- Diff: pom.xml ---
@@ -190,43 +201,6 @@
<dependencyManagement>
<dependencies>
- <!-- include version numbers for common dependencies (including common brooklyn deps) -->
--- End diff --
okay, makes sense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50760316
Comments addressed and rebased against master. Proceeding to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15229098
--- Diff: utils/rest-swagger/pom.xml ---
@@ -42,25 +42,42 @@
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
</dependency>
+ <!-- ATTN: this moves jersey-server from 1.7 to 1.12 -->
--- End diff --
What does this comment mean? Was it previously pulling in 1.7 as a transitive dependency? Do we need an excludes to stop that happening?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15409042
--- Diff: pom.xml ---
@@ -190,43 +201,6 @@
<dependencyManagement>
<dependencies>
- <!-- include version numbers for common dependencies (including common brooklyn deps) -->
--- End diff --
why is it better not to do this? i see you've included the version of these dependencies where they are declared as dependencies rather than recording these versions here, either should work, just curious why you prefer the new way...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15222927
--- Diff: camp/camp-server/pom.xml ---
@@ -101,36 +103,79 @@
</exclusion>
</exclusions>
</dependency>
-
<dependency>
<groupId>com.sun.jersey.contribs</groupId>
<artifactId>jersey-multipart</artifactId>
- <version>${jersey.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-core</artifactId>
</dependency>
<dependency>
<groupId>javax.validation</groupId>
<artifactId>validation-api</artifactId>
- <version>${validation-api.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-servlet</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty.orbit</groupId>
+ <artifactId>javax.servlet</artifactId>
</dependency>
<dependency>
<groupId>io.brooklyn</groupId>
<artifactId>brooklyn-utils-rest-swagger</artifactId>
- <version>${brooklyn.version}</version>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </dependency>
+ <!-- ATTN: this moves the dependency version from 1.9.2 to 1.9.13 -->
--- End diff --
What does this mean? Were we previously pulling in a transitive dependency of a different version? If so, do we need an `excludes`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15521802
--- Diff: pom.xml ---
@@ -190,43 +201,6 @@
<dependencyManagement>
<dependencies>
- <!-- include version numbers for common dependencies (including common brooklyn deps) -->
--- End diff --
Simply because a there were more intra-Brooklyn dependencies appearing so this list would have to grow. I thought it simpler to just declare a rule that *"intra-Brooklyn dependencies must always say `<version>${project.version}</version>` and external dependencies must only declare `<version>` in the root pom.xml."*
No strong opinion on this however, and my reasoning is not hugely compelling...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223193
--- Diff: pom.xml ---
@@ -179,6 +179,17 @@
<requirejs-maven-plugin.version>2.0.0</requirejs-maven-plugin.version>
<maven-antrun-plugin.version>1.7</maven-antrun-plugin.version>
<eclipse-m2e.version>1.0.0</eclipse-m2e.version>
+ <jetty-orbit-javax-servlet.version>3.0.0.v201112011016</jetty-orbit-javax-servlet.version>
+ <jcommander.version>1.27</jcommander.version>
--- End diff --
Again delete this; see earlier comment and https://github.com/apache/incubator-brooklyn/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223026
--- Diff: core/pom.xml ---
@@ -136,37 +129,84 @@
<dependency>
<groupId>org.freemarker</groupId>
<artifactId>freemarker</artifactId>
- <version>${freemarker.version}</version>
</dependency>
<dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>xml-apis</groupId>
+ <artifactId>xml-apis</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-core-asl</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
+ </dependency>
+ <dependency>
<groupId>mx4j</groupId>
<artifactId>mx4j-tools</artifactId>
- <version>${mx4j.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.mockwebserver</groupId>
<artifactId>mockwebserver</artifactId>
- <version>${mockwebserver.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-security</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.jayway.jsonpath</groupId>
<artifactId>json-path</artifactId>
- <version>${jsonPath.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
This shouldn't be imported! It's because of incorrect imports such as `com.beust.jcommander.internal.Lists`.
I'll create a PR, and tidy that up now. Can you then delete this dependency, before it gets merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50523323
most errors in my experience are from things being hard to understand -- which makes me want to make significant dependencies explicit, and boring ones transitive (even if they are used). guava feels boring (in this sense -- it's a great library of course!), as in i get no value from seeing it referenced from all the downstream sub-projects, and the detriment is that i can't easily see in the source which new libraries are being pulled in.
i lean more towards redeclaring transitive dependencies where the intermediate project is external, because then it is possible the dependency could change without the focus project knowing. but since these are all part of brooklyn any change to, say the dependencies of brooklyn-utils-common, will be picked up immediately in core and elsewhere. i don't see any benefit either in catching errors or scanning licenses to have dependencies included from two brooklyn projects which themselves have a dependency relationship.
but it's a minor point, the code will function fine either way (right?). and we can't easily enforce either answer (ie require code-level dependencies to be redeclared, or prevent transitive dependencies from being redeclared) so it is likely to be approximate in any case. so overall no strong feelings...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223187
--- Diff: pom.xml ---
@@ -294,6 +268,347 @@
<artifactId>plantuml</artifactId>
<version>${plantuml.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ <version>${slf4j.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-server</artifactId>
+ <version>${jersey.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.jersey.contribs</groupId>
+ <artifactId>jersey-multipart</artifactId>
+ <version>${jersey.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ <version>${commons-lang3.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-servlet</artifactId>
+ <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-core-asl</artifactId>
+ <version>${jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-mapper-asl</artifactId>
+ <version>${jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-jaxrs</artifactId>
+ <version>${jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-xc</artifactId>
+ <version>${jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-core</artifactId>
+ <version>${fasterxml.jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-annotations</artifactId>
+ <version>${fasterxml.jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ <version>${fasterxml.jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.jaxrs</groupId>
+ <artifactId>jackson-jaxrs-json-provider</artifactId>
+ <version>${fasterxml.jackson.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.wordnik</groupId>
+ <artifactId>swagger-core_2.9.1</artifactId>
+ <version>${swagger.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>com.wordnik</groupId>
+ <artifactId>swagger-jaxrs_2.9.1</artifactId>
+ <version>${swagger.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty.orbit</groupId>
+ <artifactId>javax.servlet</artifactId>
+ <version>${jetty-orbit-javax-servlet.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-security</artifactId>
+ <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-core</artifactId>
+ <version>${jersey.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
Again delete this; see earlier comment and https://github.com/apache/incubator-brooklyn/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15222962
--- Diff: core/pom.xml ---
@@ -136,37 +129,84 @@
<dependency>
<groupId>org.freemarker</groupId>
<artifactId>freemarker</artifactId>
- <version>${freemarker.version}</version>
</dependency>
<dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>xml-apis</groupId>
+ <artifactId>xml-apis</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-core-asl</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.testng</groupId>
--- End diff --
Is this not a test scope?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15521480
--- Diff: core/pom.xml ---
@@ -136,37 +129,84 @@
<dependency>
<groupId>org.freemarker</groupId>
<artifactId>freemarker</artifactId>
- <version>${freemarker.version}</version>
</dependency>
<dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>xml-apis</groupId>
+ <artifactId>xml-apis</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-core-asl</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
+ </dependency>
+ <dependency>
<groupId>mx4j</groupId>
<artifactId>mx4j-tools</artifactId>
- <version>${mx4j.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.mockwebserver</groupId>
<artifactId>mockwebserver</artifactId>
- <version>${mockwebserver.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-security</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.jayway.jsonpath</groupId>
<artifactId>json-path</artifactId>
- <version>${jsonPath.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
Noted. I will check usages of jcommander.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/85
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223138
--- Diff: core/pom.xml ---
@@ -136,37 +129,84 @@
<dependency>
<groupId>org.freemarker</groupId>
<artifactId>freemarker</artifactId>
- <version>${freemarker.version}</version>
</dependency>
<dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>xml-apis</groupId>
+ <artifactId>xml-apis</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.jackson</groupId>
+ <artifactId>jackson-core-asl</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
+ </dependency>
+ <dependency>
<groupId>mx4j</groupId>
<artifactId>mx4j-tools</artifactId>
- <version>${mx4j.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.mockwebserver</groupId>
<artifactId>mockwebserver</artifactId>
- <version>${mockwebserver.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-security</artifactId>
- <version>${jetty.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.jayway.jsonpath</groupId>
<artifactId>json-path</artifactId>
- <version>${jsonPath.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
See https://github.com/apache/incubator-brooklyn/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50471866
Looking at e.g. [1], there are folk who argue it each way. I also like the suggestion "reflect the logical project dependencies". If the project has a source-level dependency on guava then fine to declare that rather than rely on a transitive dependency pulling it in. When combined with tools like the Maven Story Teller Plugin (MSTP) [2] then this should be ok.
What I am wary of is... if we use library X that includes in its method signatures types from library Y, then I don't want us to declare a dependency on Y just because of that. If we upgrade the version of X, then we want to automatically get the correct version of Y rather than having to manually update our pom for Y as well when MSTP tells us its wrong.
@grkvlt what are your opinions here? (I also don't want this to turn into a huge debate/distraction!)
[1] http://stackoverflow.com/questions/4226756/maven-should-i-keep-or-remove-declared-dependencies-that-are-also-transitives
[2] http://confluence.highsource.org/display/MSTP/User+Guide
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50182254
#86 is now merged. :)
as promised there are now merge conflicts. :(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223199
--- Diff: software/database/pom.xml ---
@@ -40,10 +40,56 @@
<artifactId>brooklyn-software-base</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-groovy</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
Again delete this; see earlier comment and https://github.com/apache/incubator-brooklyn/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15454330
--- Diff: locations/jclouds/pom.xml ---
@@ -38,6 +38,63 @@
<dependency>
<groupId>io.brooklyn</groupId>
<artifactId>brooklyn-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-groovy</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
--- End diff --
See https://github.com/apache/incubator-brooklyn/pull/99 - still required by some projects unfortunately.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-49743176
Looks good. A few minor comments. And would appreciate if we hold off merging until #86 is merged.
@grkvlt any comments about pom.xml best practices on this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15223180
--- Diff: policy/pom.xml ---
@@ -55,7 +84,16 @@
<dependency>
<groupId>com.google.mockwebserver</groupId>
<artifactId>mockwebserver</artifactId>
- <version>${mockwebserver.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.beust</groupId>
--- End diff --
Again delete this; see earlier comment and https://github.com/apache/incubator-brooklyn/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15521602
--- Diff: utils/rest-swagger/pom.xml ---
@@ -42,25 +42,42 @@
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
</dependency>
+ <!-- ATTN: this moves jersey-server from 1.7 to 1.12 -->
--- End diff --
Similar to previous similar comment. There were conflicting versions of jersey-server used in different parts of Brooklyn. I have settled on the latest versions of those being used and unified the dependencies. In this case it's affected this project. Again the comment was intended to be a pointer for me to test that the minor version change did not have bad side effects, so I will remove this comment before rebasing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50169392
@richardcloudsoft why redeclare transitive dependencies?
it feels simpler to me e.g. if a project depends on `brooklyn-core` it gets `brooklyn-api` and `guava` from that so we should not have additional explicit dependencies in those poms.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#discussion_r15521448
--- Diff: camp/camp-server/pom.xml ---
@@ -101,36 +103,79 @@
</exclusion>
</exclusions>
</dependency>
-
<dependency>
<groupId>com.sun.jersey.contribs</groupId>
<artifactId>jersey-multipart</artifactId>
- <version>${jersey.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-core</artifactId>
</dependency>
<dependency>
<groupId>javax.validation</groupId>
<artifactId>validation-api</artifactId>
- <version>${validation-api.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-server</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-servlet</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty.orbit</groupId>
+ <artifactId>javax.servlet</artifactId>
</dependency>
<dependency>
<groupId>io.brooklyn</groupId>
<artifactId>brooklyn-utils-rest-swagger</artifactId>
- <version>${brooklyn.version}</version>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>io.brooklyn</groupId>
+ <artifactId>brooklyn-utils-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </dependency>
+ <!-- ATTN: this moves the dependency version from 1.9.2 to 1.9.13 -->
--- End diff --
camp-server was previously in an external repository, and ended up with its own set of preferred versions that did not match brooklyn. Unifying camp-server into Brooklyn has meant that some version numbers need to change so that the entire project has consistent versions of dependencies. In this case it's caused camp-server to depend on a newer version of Jackson. It's possible that this might introduce a bug but moving forward minor versions is generally safe and I have run all units tests and a smoke test with a real app. TBH I didn't mean to commit this comment, it's mostly a pointer for me for testing, and I will probably remove it when rebasing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Various dependency clean-ups
Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/85#issuecomment-50475810
@ahgittin: @aledsage's reasoning agrees with mine, with the added thought if it's sometimes better to be explicit rather than implicit. This change has resulted in one or two surprising dependencies coming to light, such as references to JUnit's `Assert` class and `com.beust.jcommander`'s `List` class. These are programming errors that detected using this practice.
More specifically, for my next step I will need to be identifying the licenses of our dependencies, and having our dependencies explicitly listed in each project will help me in the event of any problematic licenses (of which I know there will be at least two) - I will be quickly able to determine which projects may require action.
@aledsage - I am slightly concerned by your statement about library X and library Y. If the application invokes a method on library X, and that method uses a type for library Y, then the application will have to work with the library Y type and therefore its code will depend on library Y. Under this scheme, library Y would be explicitly declared. *However*, if the application uses library X but *does* *not* use the problematic method, the application code makes no references to library Y and therefore there is no dependency on library Y. Does that address your concern?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---