You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2018/01/09 00:20:45 UTC

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

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 <cl...@...>
Date:   2018-01-08T23:43:24Z

    ARTEMIS-1591 Preserve ordering on message exporter

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

    NO-JIRA fixing header copy & paste error

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

    ARTEMIS-1545 Fixing compatibility issues with topic Subscriptions

----


---

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

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

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

Posted by clebertsuconic <gi...@git.apache.org>.
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.


---

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

Posted by clebertsuconic <gi...@git.apache.org>.
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...


---

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

Posted by michaelandrepearce <gi...@git.apache.org>.
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.


---

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

Posted by clebertsuconic <gi...@git.apache.org>.
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.


---

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

Posted by dudaerich <gi...@git.apache.org>.
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?


---

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

Posted by michaelandrepearce <gi...@git.apache.org>.
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?


---

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

Posted by clebertsuconic <gi...@git.apache.org>.
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.


---

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

Posted by clebertsuconic <gi...@git.apache.org>.
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.


---