Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2035#discussion_r184039273
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
---
@@ -1849,7 +1860,9 @@ public String listSessionsAsJSON(final String
connectionID) throws Exception {
try {
List<ServerSession> sessions = server.getSessions(connectionID);
for (ServerSession sess : sessions) {
- JsonObjectBuilder obj =
JsonLoader.createObjectBuilder().add("sessionID",
sess.getName()).add("creationTime",
sess.getCreationTime()).add("consumerCount", sess.getServerConsumers().size());
+ JsonObjectBuilder obj =
JsonLoader.createObjectBuilder().add("sessionID", sess.getName())
+ .add("creationTime", sess.getCreationTime())
--- End diff --
I don't want to speak for @michaelandrepearce, but personally I find it
much easier to review PRs when all the changes in the PR are directly related.
This PR is about adding missing fields to some management methods not
increasing code legibility. In general, increasing code legibility is a good
thing, but it can be detrimental in non-obvious ways (e.g. when lots of
legibility changes make it difficult to back-port fixes to maintenance
branches) and IMO changes which are exclusively for improving legibility should
be in their own commits. Finally, "legibility" can be subjective. You risk
having your functional PR rejected by including non-functional, subjective
"improvements."
---
