[GitHub] [activemq] gemmellr commented on a change in pull request #786: Revert "[AMQ-7309] Update to jakarta.jms/jakarta.jms-api:2.0.3 (#682)"

[GitBox <gi...@apache.org>]

gemmellr commented on a change in pull request #786:
URL: https://github.com/apache/activemq/pull/786#discussion_r816851739



##########
File path: activemq-karaf/src/main/resources/features-core.xml
##########
@@ -31,6 +31,7 @@
         <bundle dependency="true">mvn:org.jvnet.jaxb2_commons/jaxb2-basics-runtime/${jaxb-basics-version}</bundle>
         <bundle dependency='true'>mvn:org.apache.servicemix.bundles/org.apache.servicemix.bundles.jaxb-impl/${jaxb-bundle-version}</bundle>
         <bundle>mvn:org.apache.commons/commons-pool2/${commons-pool2-version}</bundle>
+        <bundle dependency="true">mvn:org.apache.zookeeper/zookeeper/${zookeeper-version}</bundle>

Review comment:
       Although the original commit did remove it, putting this back for the actual release seems wrong. It seems surprising it would actually be related to the API change, so assuming it wasnt really then either omitting it here or adding another commit to take it back out would seem appropriate.

##########
File path: activemq-karaf/pom.xml
##########
@@ -32,17 +32,13 @@
 
   <properties>
     <xpp3-bundle-version>1.1.4c_5</xpp3-bundle-version>
+    <jodatime-bundle-version>2.9</jodatime-bundle-version>

Review comment:
       If removing the other bit this would seem unnecessary also.

##########
File path: activemq-karaf/src/main/resources/features-core.xml
##########
@@ -55,6 +56,7 @@
       <bundle dependency="true">mvn:org.apache.servicemix.bundles/org.apache.servicemix.bundles.jasypt-spring31/1.9.2_1</bundle>
       <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.stax-api-1.0/${servicemix.specs.version}</bundle>
       <bundle dependency="true">mvn:org.apache.servicemix.bundles/org.apache.servicemix.bundles.xpp3/${xpp3-bundle-version}</bundle>
+      <bundle dependency="true">mvn:joda-time/joda-time/${jodatime-bundle-version}</bundle>

Review comment:
       I'd wonder if this is actually needed (like the zookeeper, doesnt seem like it is should actually be related to the API change even if it was in the commit), and so again assuming it isnt would also probably omit it or add a second commit removing it (and related property) again too.




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