----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61701/#review183621 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java Lines 2014-2028 (original), 2013-2027 (patched) <https://reviews.apache.org/r/61701/#comment259690> Is there a concensus on whether this style of parameter labeling is good or if we should remove it? I personally don't like it, especially since IntelliJ fills in the labels for you when you enter a raw value instead of passing a parameter. geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/lifecycle/GfshStatusCommandsIntegrationTest.java Lines 19-37 (original), 19-39 (patched) <https://reviews.apache.org/r/61701/#comment259688> Hat trick for my nit-picks. * There are a couple unused imports (`InetAddress` and `ClassRule`). * `final private static` is preferred to be `private static final`. (There's an IntelliJ inspection for this that is off by default.) * That `// private int port` could be removed, along with the `// port = getRandomAvailablePort(SOCKET);` below geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java Lines 47-52 (original), 47-52 (patched) <https://reviews.apache.org/r/61701/#comment259689> A man after my own heart. - Patrick Rhomberg On Aug. 22, 2017, 9:28 p.m., Ken Howe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61701/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2017, 9:28 p.m.) > > > Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, > and Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > Updated tests for changes in the error constructors for ServerState and > LocatorState. > > Minor spelling corrections. > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java > 3a98373938e3de21da6badcf460dae3648218ac6 > geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java > 83c1ab533e3dea323a8a99f7002b9464a54dfc25 > geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java > ae64691605130c9b212a3a33bb65ae37b28af02b > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/lifecycle/GfshStatusCommandsIntegrationTest.java > dd5841f4cffca38da07a11f381cf4174d7264349 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > e7f17ef208a1708f385c7c4041affb70fd309a4c > > > Diff: https://reviews.apache.org/r/61701/diff/3/ > > > Testing > ------- > > Precheckin from earlier ran green. > > Re-running precheckin with this additional refactoring. > > > Thanks, > > Ken Howe > >
