[GitHub] activemq-artemis pull request #1854: [ARTEMIS-1666] List of prepared transac...

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

[GitHub] activemq-artemis pull request #1854: [ARTEMIS-1666] List of prepared transac...

michaelandrepearce-2
GitHub user yersan opened a pull request:

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

    [ARTEMIS-1666] List of prepared transaction details returns Object.toString() instead of Json string

    Issue: https://issues.apache.org/jira/browse/ARTEMIS-1666
    JBEAP Issue: https://issues.jboss.org/browse/JBEAP-14177

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

    $ git pull https://github.com/yersan/activemq-artemis bugs/ARTEMIS-1666

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

    https://github.com/apache/activemq-artemis/pull/1854.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 #1854
   
----
commit ecc13750929f7969c7cd36da357af7273a88f607
Author: Yeray Borges <yborgess@...>
Date:   2018-02-07T14:56:37Z

    [ARTEMIS-1666] List of prepared transaction details returns Object.toString() instead of Json string

----


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    Typically issues like this would be fixed on the 'master' branch first and then back-ported to any other branches to avoid regressions.  Although the JMSServerManager is deprecated on 'master' I think this is still worth fixing there first and then back-porting it to the 1.x branch.


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    If you don't want to do it let me know, and I'll do it.  No worries.
   
    In any case, I think this PR should be closed.


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user yersan commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    @jbertram will it work for you if I create a new one PR on master branch and let this one open for 1.x?


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    Ideally there would be a new PR on master and then a cherry-pick of that same commit onto 1.x (which would preclude this PR from staying open).


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user yersan commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    Ok, I'm going to close this one and create a new one for master and for 1.x (cherry-picked from master once it is merged)


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user yersan commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    @jbertram Note that there is no JMSServerControlTest test on master branch, but it exists on 1.x. This test can be used to cover this issue against regression, how do you usually deal with these cases? will it be fine if, when this issue is cherry picked for 1.x, we add a new commit including the test case into the PR over 1.x?


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    So forget what I said before.  I'll merge this PR and then send a different one upstream to deal with this issue in a different way given the management objects have changed so substantially here.


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    Although this has been merged the PR won't be closed automatically (something funny going on in the automation).  @yersan, please close this PR manually.


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

[GitHub] activemq-artemis issue #1854: [ARTEMIS-1666] List of prepared transaction de...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user yersan commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1854
 
    @jbertram Done, thanks!


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

[GitHub] activemq-artemis pull request #1854: [ARTEMIS-1666] List of prepared transac...

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

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


---