[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-26 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 @clebertsuconic It is fixing a test about message expiring indeed :) ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-26 Thread clebertsuconic
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 this may be fixing a test actually ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread clebertsuconic
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 I see... Can I ask you a favor... if a test for your change doesn't make sense, can you explicitly add a comment to your commit message saying "a test is not needed"?

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 @cshannon Thanks to have taken a look!! :+1: ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread cshannon
Github user cshannon commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 Ah yep you are right so there is no change needed. ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 @cshannon Good point! I've checked it and seems that it already implements just the new version of the method...it is correct? ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread cshannon
Github user cshannon commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 @franz1981 - You should probably also update the tests as well. The MethodCalledVerifier class implements ActiveMQServerPlugin in the integration tests project and should probably have i

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread cshannon
Github user cshannon commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 Oops, yeah this is the correct fix. The new default method would call off to the old one as it's default if nothing overrides it to stay backwards compatible. And then with Artemis 3.0.

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread clebertsuconic
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 you would need a major release to remove these. ---

[GitHub] activemq-artemis issue #2044: ARTEMIS-1829 Remove deprecated plugin's messag...

2018-04-25 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2044 @cshannon wdyt? I will need your opinion because I don't know how you've chosen to deal with deprecated notifications :smile: ---