From dev-return-65281-archive-asf-public=cust-asf.ponee.io@activemq.apache.org Wed Apr 25 14:27:38 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 1765E180676 for ; Wed, 25 Apr 2018 14:27:37 +0200 (CEST) Received: (qmail 20378 invoked by uid 500); 25 Apr 2018 12:27:36 -0000 Mailing-List: contact dev-help@activemq.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@activemq.apache.org Delivered-To: mailing list dev@activemq.apache.org Received: (qmail 20355 invoked by uid 99); 25 Apr 2018 12:27:36 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 25 Apr 2018 12:27:36 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 3180EF32FD; Wed, 25 Apr 2018 12:27:36 +0000 (UTC) From: jbertram To: dev@activemq.apache.org Reply-To: dev@activemq.apache.org References: In-Reply-To: Subject: [GitHub] activemq-artemis pull request #2035: [ARTEMIS-1819] Missing fields on listAl... Content-Type: text/plain Message-Id: <20180425122736.3180EF32FD@git1-us-west.apache.org> Date: Wed, 25 Apr 2018 12:27:36 +0000 (UTC) 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 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." ---