You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/01/28 18:16:10 UTC

[GitHub] [activemq-artemis] ehsavoie opened a new pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

ehsavoie opened a new pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421


   * using batavia and eclipse transformer to transform JMS code to JakartaEE.
   * adding a test with JakartaEE
   
   Jira: https://issues.apache.org/jira/browse/ARTEMIS-3080


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit merged pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#issuecomment-769779446


   @clebertsuconic I merged your changes. All should be up to date.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#issuecomment-769781688


   However the build is not working on my CI. 
   
   Look at the build result on GitHub actions. 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569565845



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,95 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis Jakarta Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+            <!-- needed for the maven reactor -->
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-core-client</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-commons</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-selector</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <scope>runtime</scope>

Review comment:
       I would still list it as a compile scoped, for the knock on effect, and taking into account that it will be compile scoped in future if say the module stopped being transformed from the artemis-jms-client module. As I say, depending on artemis-jms-client gets you the required JMS API. It seems reasonable using artemis-jakarta-client gets you the required Jakarta Messaging API.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r566317273



##########
File path: tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/JmsReplyToQueueTest.java
##########
@@ -44,6 +46,7 @@ public static Collection getParameters() {
       combinations.add(new Object[]{SNAPSHOT, ONE_FIVE, SNAPSHOT});
       combinations.add(new Object[]{SNAPSHOT, SNAPSHOT, ONE_FIVE});
       combinations.add(new Object[]{SNAPSHOT, SNAPSHOT, SNAPSHOT});
+      combinations.add(new Object[]{JAKARTAEE, JAKARTAEE, JAKARTAEE});

Review comment:
       since you added it here, can you add the other combinations? 
   
   it should be possible to mix producer and consumers from JAKARTA ONE_FIVE AND SNAPSHOT?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568037573



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       I got that the dep is needed for reactor ordering, but I dont see it makes sense that the produced artemis-jakarta-client would seemingly have no dependencies specified. If so how is someone meant to actually use it, i.e how do they know what the rest of the classpath required for the client is actually meant to be in order to successfully use it? Depend on both client modules and overlook the extra dep(s) (I guess at least the API could be manually excluded)?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569294822



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,95 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis Jakarta Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+            <!-- needed for the maven reactor -->
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-core-client</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-commons</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-selector</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <scope>runtime</scope>

Review comment:
       I think the API should be compile scoped. A non-transformed implementation wouldn't get away without it being available at compile time. Using this module as a dep with it only specified at runtime scope means you must provide the API yourself at compile time (which isn't actually bad in itself, but still likely to be unexpected/confusing to many and another difference to artemis-jms-client).
   
   Presumably the transformation does have it at compile time already, due to using the dependency-managed 'provided' scope from the parent making it available? Perhaps that should also be changed to not say provided?

##########
File path: pom.xml
##########
@@ -442,7 +454,48 @@
             <version>${geronimo.jms.2.spec.version}</version>
             <!-- License: Apache 2.0 -->
          </dependency>
-
+         <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <version>${jakarta.jms-api.version}</version>
+            <!-- License: EPL 2.0 -->
+           <scope>provided</scope>

Review comment:
       Related to other comment, perhaps this scope definition should be removed?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568117616



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       Well ten a bom would make sense somehow. The goal was to provide a drop in replacement jar. Somehow, all the dependencies of the JMS client are required except for the API change. I'm not sure the goal of a pom is to provide a runtime classpath ... Maybe we could add some doc on how to use it ? Or a bom or a jakarta all client ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568005927



##########
File path: pom.xml
##########
@@ -1816,7 +1874,26 @@
             </configuration>
          </plugin>
 
-
+         <plugin>
+             <groupId>org.wildfly.extras.batavia</groupId>
+             <artifactId>transformer-tools-mvn</artifactId>
+             <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>${version.batavia}</version>
+                    </dependency>
+                </dependencies>
+         </plugin>

Review comment:
       That way I don't have to redefine the dependency in each sub module




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568025579



##########
File path: pom.xml
##########
@@ -1816,7 +1874,26 @@
             </configuration>
          </plugin>
 
-
+         <plugin>
+             <groupId>org.wildfly.extras.batavia</groupId>
+             <artifactId>transformer-tools-mvn</artifactId>
+             <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>${version.batavia}</version>
+                    </dependency>
+                </dependencies>
+         </plugin>

Review comment:
       Moved back to the initial build configuration




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568474409



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       I'd say it is often expected a pom will provide the needed classpath, and would likely be in this case given historic usage. That is to say I expect few people declaring a dependency on artemis-jms-client are surprised it yields one, and guess a good proportion who might later try artemis-jakarta-client would be surprised it didn't similarly. I'm not sure how a bom would help here, unless the thought is someone could manually copy the needed dep definitions from its contents? An 'all' jar could certainly be an option (though I don't like those personally).
   
   Perhaps its fine as a first step, but yes some doc (e.g at least a README in the module) on how to use it would seem needed, I think people would struggle to use it as-is otherwise.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569574404



##########
File path: pom.xml
##########
@@ -442,7 +454,48 @@
             <version>${geronimo.jms.2.spec.version}</version>
             <!-- License: Apache 2.0 -->
          </dependency>
-
+         <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <version>${jakarta.jms-api.version}</version>
+            <!-- License: EPL 2.0 -->
+           <scope>provided</scope>

Review comment:
       Perhaps, though I'd consider it somewhat seperate than the main-client-API case I was interested in as it pertains to the artemis-jakarta-client module. The comparible JMS spec entry to that case, right above it, does not have use a 'provided' scope (though it is expected to be compile scope...which is somewhat the point here).
   
   I do personally typically omit scopes from dependencyManagement though and mainly just use it for versioning, I think its clearer to have the specific modules that want non-compile scopes indicate that themselves.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r566834305



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,71 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.wildfly.extras.batavia</groupId>
+                <artifactId>transformer-tools-mvn</artifactId>
+                <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                        <configuration>
+                            <inputFile>${project.basedir}/../artemis-jms-client/target/classes/</inputFile>
+                            <outputFolder>${project.build.outputDirectory}</outputFolder>
+                        </configuration>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>1.0.5.Final</version>

Review comment:
       The tool version looks to be dependency-managed in the parent, but this seemingly-related dep version isnt. Should it be, and then this removed? It also looks to be used in multiple places which should probably be ensured are consistent.

##########
File path: artemis-jakarta-ra/pom.xml
##########
@@ -0,0 +1,72 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+   <name>ActiveMQ Artemis JakartaEE RAR</name>
+   <modelVersion>4.0.0</modelVersion>
+
+   <parent>
+      <groupId>org.apache.activemq</groupId>
+      <artifactId>artemis-pom</artifactId>
+      <version>2.17.0-SNAPSHOT</version>
+   </parent>
+
+   <artifactId>artemis-jakarta-ra</artifactId>
+   <packaging>jar</packaging>
+
+   <properties>
+      <activemq.basedir>${project.basedir}/..</activemq.basedir>
+   </properties>
+
+
+  <dependencies>
+      <dependency>
+          <groupId>org.apache.activemq</groupId>
+          <artifactId>artemis-ra</artifactId>
+          <version>${project.version}</version>
+          <scope>provided</scope>
+      </dependency>
+  </dependencies>
+
+  <build>
+      <plugins>
+          <plugin>
+              <groupId>org.wildfly.extras.batavia</groupId>
+              <artifactId>transformer-tools-mvn</artifactId>
+              <executions>
+                  <execution>
+                      <id>transform</id>
+                      <phase>process-classes</phase>
+                      <goals>
+                          <goal>transform-classes</goal>
+                      </goals>
+                      <configuration>
+                          <inputFile>${project.basedir}/../artemis-ra/target/classes/</inputFile>
+                          <outputFolder>${project.build.outputDirectory}</outputFolder>
+                      </configuration>
+                  </execution>
+              </executions>
+              <dependencies>
+                  <dependency>
+                      <groupId>org.wildfly.extras.batavia</groupId>
+                      <artifactId>transformer-impl-eclipse</artifactId>
+                      <version>1.0.5.Final</version>

Review comment:
       Same as other comment

##########
File path: artemis-jakarta-service-extensions/pom.xml
##########
@@ -0,0 +1,71 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+   <modelVersion>4.0.0</modelVersion>
+
+   <parent>
+      <groupId>org.apache.activemq</groupId>
+      <artifactId>artemis-pom</artifactId>
+      <version>2.17.0-SNAPSHOT</version>
+   </parent>
+
+   <artifactId>artemis-jakarta-service-extensions</artifactId>
+   <packaging>jar</packaging>
+   <name>ActiveMQ Artemis JakartaEE Service Extensions</name>
+
+   <properties>
+      <activemq.basedir>${project.basedir}/..</activemq.basedir>
+   </properties>
+
+   <dependencies>
+      <dependency>
+          <groupId>org.apache.activemq</groupId>
+          <artifactId>artemis-service-extensions</artifactId>
+          <version>${project.version}</version>
+          <scope>provided</scope>
+      </dependency>
+   </dependencies>
+
+   <build>
+      <plugins>
+         <plugin>
+            <groupId>org.wildfly.extras.batavia</groupId>
+            <artifactId>transformer-tools-mvn</artifactId>
+            <executions>
+               <execution>
+                   <id>transform</id>
+                   <phase>process-classes</phase>
+                   <goals>
+                      <goal>transform-classes</goal>
+                   </goals>
+                   <configuration>
+                      <inputFile>${project.basedir}/../artemis-service-extensions/target/classes/</inputFile>
+                      <outputFolder>${project.build.outputDirectory}</outputFolder>
+                   </configuration>
+                </execution>
+             </executions>
+             <dependencies>
+                <dependency>
+                    <groupId>org.wildfly.extras.batavia</groupId>
+                    <artifactId>transformer-impl-eclipse</artifactId>
+                    <version>1.0.5.Final</version>

Review comment:
       Same as other comment

##########
File path: artemis-jakarta-server/pom.xml
##########
@@ -0,0 +1,71 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-server</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Server</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-server</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.wildfly.extras.batavia</groupId>
+                <artifactId>transformer-tools-mvn</artifactId>
+                <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                        <configuration>
+                            <inputFile>${project.basedir}/../artemis-jms-server/target/classes/</inputFile>
+                            <outputFolder>${project.build.outputDirectory}</outputFolder>
+                        </configuration>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>1.0.5.Final</version>

Review comment:
       Same as other 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569468747



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,95 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis Jakarta Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+            <!-- needed for the maven reactor -->
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-core-client</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-commons</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-selector</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <scope>runtime</scope>

Review comment:
       The transform does bytecode transformation directly, I'm not sure the API is required as I don't see it in the batavia nor in the eclipse transformer poms. I think we could do without. I don't think there is any compilation here.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568006600



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>

Review comment:
       correct :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r566391372



##########
File path: tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/JmsReplyToQueueTest.java
##########
@@ -44,6 +46,7 @@ public static Collection getParameters() {
       combinations.add(new Object[]{SNAPSHOT, ONE_FIVE, SNAPSHOT});
       combinations.add(new Object[]{SNAPSHOT, SNAPSHOT, ONE_FIVE});
       combinations.add(new Object[]{SNAPSHOT, SNAPSHOT, SNAPSHOT});
+      combinations.add(new Object[]{JAKARTAEE, JAKARTAEE, JAKARTAEE});

Review comment:
       There are 4 tests doing the same deal.. all of them should mix Jakarta and JMS on these versions IMO. it would be a good thing to do.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569294822



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,95 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis Jakarta Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+            <!-- needed for the maven reactor -->
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-core-client</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-commons</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-selector</artifactId>
+            <version>${project.version}</version>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <scope>runtime</scope>

Review comment:
       I think the API should be compile scoped. A non-transformed implementation wouldn't get away without it being available at compile time. Using this module as a dep with it only specified at runtime scope means you must provide the API yourself at compile time (which isn't actually bad in itself, but still likely to be unexpected/confusing to many and another difference to artemis-jms-client).
   
   Presumably the transformation does have it at compile time already, due to using the dependency-managed 'provided' scope from the parent making it available? Perhaps that should also be changed to not say provided?
   (Or perhaps the transform process just doesnt use the API at all...I've not looked at it hehe).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569236255



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       Added the dependencies in the runtime scope




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#issuecomment-769390339


   There was also a variable missing in the pom, which I added as part of the PR I sent you


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r569470074



##########
File path: pom.xml
##########
@@ -442,7 +454,48 @@
             <version>${geronimo.jms.2.spec.version}</version>
             <!-- License: Apache 2.0 -->
          </dependency>
-
+         <dependency>
+            <groupId>jakarta.jms</groupId>
+            <artifactId>jakarta.jms-api</artifactId>
+            <version>${jakarta.jms-api.version}</version>
+            <!-- License: EPL 2.0 -->
+           <scope>provided</scope>

Review comment:
                <dependency>
               <groupId>org.apache.geronimo.specs</groupId>
               <artifactId>geronimo-annotation_1.2_spec</artifactId>
               <version>${geronimo-annotation_1.2_spec.version}</version>
               <scope>provided</scope>
            </dependency> would be similar




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r567891785



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>

Review comment:
       Isn't the name just Jakarta Messaging? (i.e not JakartaEE Messaging)

##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       Feels a bit odd that this module doesn't seem to actually end up with a dependency on the new API, shouldnt it? Actually, what about all the other client deps too for that matter, aren't they are omitted also...how would someone use this module currently?

##########
File path: pom.xml
##########
@@ -1816,7 +1874,26 @@
             </configuration>
          </plugin>
 
-
+         <plugin>
+             <groupId>org.wildfly.extras.batavia</groupId>
+             <artifactId>transformer-tools-mvn</artifactId>
+             <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>${version.batavia}</version>
+                    </dependency>
+                </dependencies>
+         </plugin>

Review comment:
       Defining the execution here will mean the plugin runs (though doesn't necessarily do anything) for every module in the build. Is that needed, seems a bit ugly?
   
   I dont recall it being that way last week when skimming the changes. If this came about to try and have the transformer-impl-eclipse dep version be consistent, just using the property in the previous places the version was specified would seem preferable if there isnt a specific reason this approach was needed.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568005297



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       The produced jar should 'replace' the artemis-jms-client jar in the classpath. This dependency is there to 'force' the reactor to build the jms client first.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#issuecomment-769390107


   I sent a PR to your branch, however the tests are failing.
   
   perhaps we would need to remove some of the combinations?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r568474409



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,64 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>

Review comment:
       I'd say it is often expected a pom will provide the needed classpath, and would likely be in this case given historic usage. That is to say I expect few people declaring a dependency on artemis-jms-client are surprised it yields one, and guess a good proportion who might later try artemis-jakarta-client would be surprised it didn't similarly. I'm not sure how a bom would help here, unless the thought is someone could manually copy the needed dep definitions from its contents? An 'all' jar could certainly be an option (though I don't like those personally).
   
   Perhaps its fine as a first step, but yes some doc (e.g at least a README in the module) on how to use it would seem needed, I think people would struggle to use it as-is otherwise.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3421: [ARTEMIS-3080]: Provide JakartaEE 9 artefacts.

Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3421:
URL: https://github.com/apache/activemq-artemis/pull/3421#discussion_r566929004



##########
File path: artemis-jakarta-client/pom.xml
##########
@@ -0,0 +1,71 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.activemq</groupId>
+        <artifactId>artemis-pom</artifactId>
+        <version>2.17.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>artemis-jakarta-client</artifactId>
+    <packaging>jar</packaging>
+    <name>ActiveMQ Artemis JakartaEE Messaging Client</name>
+
+    <properties>
+        <activemq.basedir>${project.basedir}/..</activemq.basedir>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.activemq</groupId>
+            <artifactId>artemis-jms-client</artifactId>
+            <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.wildfly.extras.batavia</groupId>
+                <artifactId>transformer-tools-mvn</artifactId>
+                <executions>
+                    <execution>
+                        <id>transform</id>
+                        <phase>process-classes</phase>
+                        <goals>
+                            <goal>transform-classes</goal>
+                        </goals>
+                        <configuration>
+                            <inputFile>${project.basedir}/../artemis-jms-client/target/classes/</inputFile>
+                            <outputFolder>${project.build.outputDirectory}</outputFolder>
+                        </configuration>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.wildfly.extras.batavia</groupId>
+                        <artifactId>transformer-impl-eclipse</artifactId>
+                        <version>1.0.5.Final</version>

Review comment:
       we should use the same property I guess. For some reason I couldn't make it work by defining it in the plugin management part of the parent




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org