You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jbertram (via GitHub)" <gi...@apache.org> on 2023/10/17 15:07:13 UTC
[PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
jbertram opened a new pull request, #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr merged PR #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650#issuecomment-1771146566
I dont believe the artemis-distribution pom depends on a lot of what justins change did, so it cant be enforcing the same constraints, i.e its also just working by luck (as the original works if you dont use a parallel build). It also still pulls in all those dependencies and so doesnt address my feedback on that side either.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650#discussion_r1363765234
##########
artemis-features/pom.xml:
##########
@@ -49,6 +49,58 @@
<type>kar</type>
<scope>provided</scope>
</dependency>
+
+ <!-- these project-specific dependencies are referenced in features.xml and therefore should be declared here -->
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-quorum-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-server-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-core-client-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-jms-client-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-amqp-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-stomp-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-mqtt-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-hqclient-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-hornetq-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-openwire-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
Review Comment:
I'm guessing you are adding this for build-ordering reasons? If so, it may be wise to structure things so that they apply build constraints without really altering the user-facing dependencies, e.g excluding all the deps (and perhaps making them optional also).
The fact these have never been here mean they clearly arent needed by the actual feature usage. Their new presence however could mean they are now all depended on for things touching the pom, and may need to resolve all those modules and their dependencies, even if the feature being used doesnt need them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650#issuecomment-1771118786
@gemmellr I just pushed -f on Justin's branch to depend on the Artemis-distribution pom instead. if you could check this please?
I tested it here and it worked quite alright.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-4461 make implicit dependencies explicit in artemis-features [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4650:
URL: https://github.com/apache/activemq-artemis/pull/4650#discussion_r1365600691
##########
artemis-features/pom.xml:
##########
@@ -49,6 +49,58 @@
<type>kar</type>
<scope>provided</scope>
</dependency>
+
+ <!-- these project-specific dependencies are referenced in features.xml and therefore should be declared here -->
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-quorum-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-server-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-core-client-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-jms-client-osgi</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-amqp-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-stomp-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-mqtt-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-hqclient-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-hornetq-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-openwire-protocol</artifactId>
+ <version>${project.version}</version>
+ </dependency>
Review Comment:
@gemmellr by definition we should only build this component (features) after the distribution.
So, I guess the real missing dependency here is the Artemis-distribution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org