[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
GitHub user clebertsuconic opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1758

    More fixes around compatibility

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/clebertsuconic/activemq-artemis ARTEMIS-1545

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/1758.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 #1758
   
----
commit 48de387095dc947fe3a391d034fdc8ed0e64bddf
Author: Clebert Suconic <clebertsuconic@...>
Date:   2018-01-08T23:43:24Z

    ARTEMIS-1591 Preserve ordering on message exporter

commit 04ea73da87a25bb6811452f4277d71c435f41f3e
Author: Clebert Suconic <clebertsuconic@...>
Date:   2018-01-09T00:06:15Z

    NO-JIRA fixing header copy & paste error

commit da631ab6175ba8466e0549934418ad4fa446e3b7
Author: Clebert Suconic <clebertsuconic@...>
Date:   2018-01-08T20:11:52Z

    ARTEMIS-1545 Fixing compatibility issues with topic Subscriptions

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1758: More fixes around compatibility

jgoodyear-2
Github user dudaerich commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1758
 
    +100 for the export/import journal compatibility tests and for the compatibility tests in general. Thanks @clebertsuconic
   
    I noticed that there is no difference between export.groovy, export1X.groovy and import.groovy, import1X.groovy. Is it intention to have separate implementations even though they are the same?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160420063
 
    --- Diff: tests/compatibility-tests/src/main/resources/exportimport/import1X.groovy ---
    @@ -0,0 +1,11 @@
    +import org.apache.activemq.artemis.cli.commands.ActionContext
    --- End diff --
   
    License header missing, should this not be picked up by RAT report? Do we need to make rat look at groovy files?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160420445
 
    --- Diff: tests/compatibility-tests/src/main/resources/servers/hornetqServer.groovy ---
    @@ -19,7 +23,7 @@ import org.hornetq.core.config.impl.ConfigurationImpl
     import org.hornetq.core.remoting.impl.netty.NettyAcceptorFactory
     import org.hornetq.core.remoting.impl.netty.TransportConstants
     import org.hornetq.jms.server.config.impl.JMSConfigurationImpl
    -import org.hornetq.jms.server.config.impl.JMSQueueConfigurationImpl
    +import org.hornetq.jms.server.config.impl.*
    --- End diff --
   
    nit: but can this expanded import, especially as still explicitly importing other class's from the same package.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160433884
 
    --- Diff: tests/compatibility-tests/src/main/resources/servers/hornetqServer.groovy ---
    @@ -19,7 +23,7 @@ import org.hornetq.core.config.impl.ConfigurationImpl
     import org.hornetq.core.remoting.impl.netty.NettyAcceptorFactory
     import org.hornetq.core.remoting.impl.netty.TransportConstants
     import org.hornetq.jms.server.config.impl.JMSConfigurationImpl
    -import org.hornetq.jms.server.config.impl.JMSQueueConfigurationImpl
    +import org.hornetq.jms.server.config.impl.*
    --- End diff --
   
    I didn't care much as this is a groovy script. but i will expand it.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160433959
 
    --- Diff: tests/compatibility-tests/src/main/resources/exportimport/import1X.groovy ---
    @@ -0,0 +1,11 @@
    +import org.apache.activemq.artemis.cli.commands.ActionContext
    --- End diff --
   
    It was picked actually.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160434008
 
    --- Diff: tests/compatibility-tests/src/main/resources/exportimport/import1X.groovy ---
    @@ -0,0 +1,11 @@
    +import org.apache.activemq.artemis.cli.commands.ActionContext
    --- End diff --
   
    the reason is failed...


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160434660
 
    --- Diff: tests/compatibility-tests/src/main/resources/exportimport/import1X.groovy ---
    @@ -0,0 +1,11 @@
    +import org.apache.activemq.artemis.cli.commands.ActionContext
    --- End diff --
   
    @dudaerich the reason we have export / export1X is because the package names are different.
   
    I could have fixed it differently with a different script just to instantiate the export.. but the script was so small that I decided just to have different versions for each branch.
   
   
    the script wouldn't load (or compile) if I had any sort of if inside.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1758#discussion_r160436610
 
    --- Diff: tests/compatibility-tests/src/main/resources/servers/hornetqServer.groovy ---
    @@ -19,7 +23,7 @@ import org.hornetq.core.config.impl.ConfigurationImpl
     import org.hornetq.core.remoting.impl.netty.NettyAcceptorFactory
     import org.hornetq.core.remoting.impl.netty.TransportConstants
     import org.hornetq.jms.server.config.impl.JMSConfigurationImpl
    -import org.hornetq.jms.server.config.impl.JMSQueueConfigurationImpl
    +import org.hornetq.jms.server.config.impl.*
    --- End diff --
   
    I will keep this one as is actually... I have no help from the IDE on what classes to import as this is hornetq... (It wasn't part of my change now anyways).
   
   
    (this is the reason I'm using groovy.. I can tweak the classloader.. and have no direct dependency on the project.. totally optional. even the scan is done by a plugin.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1758: More fixes around compatibility

jgoodyear-2
In reply to this post by jgoodyear-2
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1758


---