Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
Thanks @michaelandrepearce , @mtaylor
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox all merged now, thanks for this!
---
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox @michaelandrepearce This looks reasonable to me. Nice work on
catching these issues.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@mtaylor Ill look to merge this later today, if you wanted to check it
before hand?
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
Thanks, @michaelandrepearce @pgfox for the details provided I will keep
this in mind for future.
---
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
- added in @michaelandrepearce commit to the PR
- fixed a few minor check style infringements
- added testing to exercise some of the sorting code
I did not squash the
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox please do, I also didnât check for checkstyle and all that stuff.
---
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@michaelandrepearce Thanks Mike, I updated the PR with your commit. I
will do a test run tonight on the new PR.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox have PR'd to your branch
https://github.com/pgfox/activemq-artemis/pull/1 suggested change, tbh it be
even better if we can enum this up somehow, as there some field
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
API names of stuff does not have to match internal names of stuff.
Classical internal model mapped to a DTO.
Eg to make the json, internal fields are mapped into json
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox
For Sorting to work the JSON field name change was necessary as per me. If
you look closely you will notice that these field name as actually used to call
the individual
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox looks good thanks, will merge in a day or two if thats ok? just like
to leave it a bit of time just for others to get a chance to skim over it
before merging ( nudge
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
- reverted JSON fields to origin names before #1628
- updated *.js to reflect field name reverts.
---
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
Hi @RaiSaurabh, great work on #1628.
One minor point, the JSON field name changes as part of that PR causes a
test to fail. That sparked the conversation above with Mike. It is not
Github user pgfox commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@michaelandrepearce Thanks Mike,
I agree about the field names in the JSON objects; IMHO they are really
part of the external interface so should not be changed unless there is a
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1696
@pgfox Good catch, i think this has picked up a regression in the code, the
test has done its job in detecting that change, so i don't think we should
change the test, but
16 matches
Mail list logo
