Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
Looks good. I give it a try this evening when back home.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben I have merged, after I reverted some of the bad code I made you
do. Can you check please?
And sorry about that.. I really thought the ProtocolTracker was externally
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@clebertsuconic That's fine with me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben I actually just asked this on the dev-list
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben I thought ProtocolTracker was part of the API. it's for internal
use only.
We could have renamed it as much as we wanted. Since I'm the one who messed
up I will fix
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
I love checkstyle. :) I thought about the actual implementation and did
some small refactorings. Now I think it fits best. But we may think about
synchronizing in callback as they are
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
sounds good
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
Have to fix a bug introduced by refactoring. I think inner classes are
looking better than an anonymous one. It might be better to add components to
ProtocolTrackerCallBackImpl too make it
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben I understand it works now. I was just trying to be more generic on
the implementation and support future needs.
---
If your project is set up for it, you can reply to
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
A bit more this way? IMO I did not see why to change the method to
addDataSource. It could be only one and this one is a dependency which is
mandatory for broker startup.
---
If your
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
Rebase is done :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben can you rebase against master... and push -f at least?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
let me think about the class name...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user graben commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
Well, I think it must be the same callback to avoid the protocol tracker to
start the broker if the datasource is not bounded yet. It's just another
"mandatory" dependency. I might rename
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben ^^^ (<<< just to send you a notification)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
There is one change that is wrong here. You are using the
ProtocolTrackerCallback to admin DataSources. Can you separate this into a
separate class? DataSourceTrackerCallback?
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@clebertsuconic great. It looks like @graben has addressed your comments.
If you're happy I'll merge this?
---
If your project is set up for it, you can reply to this email and have
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@mtaylor that merge script we always use will fix it. I already tried it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/839
@graben could you please squash you please rebase on top of master vs using
the merge commit.
---
If your project is set up for it, you can reply to this email and have your
reply
19 matches
Mail list logo
