----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59905/#review177324 -----------------------------------------------------------
All of my issues are nitpicks. I'm sorry. But only a little. geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java Lines 304-306 (original) <https://reviews.apache.org/r/59905/#comment250861> I am amused by this function and am a little sad to see it go. It sounds like something a cartoon hero would say. "Complete Super-Advanced Combat Maneuver Alpha!" geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java Lines 2369 (patched) <https://reviews.apache.org/r/59905/#comment250857> To pick a nit: it looks like this file only pads newlines to group w.r.t. top-level commands. Maybe remove this empty line? geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java Lines 2522 (patched) <https://reviews.apache.org/r/59905/#comment250858> Ditto above nitpick: drop this empty line? geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java Line 278 (original), 260 (patched) <https://reviews.apache.org/r/59905/#comment250862> Trivial, but you could correct the typo in these test names while you're in here. geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java Lines 296-298 (original), 278-280 (patched) <https://reviews.apache.org/r/59905/#comment250863> This is dead and can be removed? geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java Line 20 (original), 20 (patched) <https://reviews.apache.org/r/59905/#comment250865> Unused import. geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java Lines 18-34 (patched) <https://reviews.apache.org/r/59905/#comment250866> Some of these have gone stale. `assertThat`, `RemoteExecutionStrategy`, and `CommandMarker` appear unused. - Patrick Rhomberg On June 8, 2017, 1:48 a.m., Jared Stewart wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59905/ > ----------------------------------------------------------- > > (Updated June 8, 2017, 1:48 a.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-2933: Add jmx-manager-hostname-for-clients for startLocator and > startServer > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java > df16e9b9e9f71f21ef7c8ac84267a9858ee4298c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java > 4c668b6813de71aae9e3e46c82a5c231a41c1f6a > > geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java > 9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce > > geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java > c2c6e1425d71af9d2ea59046b17afd70ad30dd68 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java > c5ff6b6a51a0bd866d62e15dfce13938c87ecf6b > > geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java > a122de048eb75ca448155541e2266c872c98904d > > geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserConverterTest.java > 1ff60d67a7dbfe582e632c116ba83d471211de45 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java > ab6dc3d10a6235af0ca6fc5164cac5d3c2419cdd > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshParserRule.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/59905/diff/1/ > > > Testing > ------- > > Precheckin passed (other than 1 known flaky) > > > Thanks, > > Jared Stewart > >
