----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59323/#review175280 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java Line 224 (original), 224 (patched) <https://reviews.apache.org/r/59323/#comment248725> Looks like this comment may have been changed accidentally. Actually, since we're already looking around in this class, I think all of these "Not yet used" method comments should be deleted. geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java Line 214 (original), 214 (patched) <https://reviews.apache.org/r/59323/#comment248726> Delete this comment geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java Line 504 (original), 507 (patched) <https://reviews.apache.org/r/59323/#comment248728> There is no need to pass the empty string into this constructor, `new StringBuilder();` will suffice. geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java Line 157 (original) <https://reviews.apache.org/r/59323/#comment248745> Did you intended to remove these assertions? geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java Line 194 (original) <https://reviews.apache.org/r/59323/#comment248746> Did you intended to remove these assertions? geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java Line 165 (original) <https://reviews.apache.org/r/59323/#comment248747> Did you intended to remove these assertions? geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java Line 280 (original) <https://reviews.apache.org/r/59323/#comment248748> Did you intended to remove these assertions? - Jared Stewart On May 16, 2017, 11:25 p.m., Patrick Rhomberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59323/ > ----------------------------------------------------------- > > (Updated May 16, 2017, 11:25 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, > and Swapnil Bawaskar. > > > Repository: geode > > > Description > ------- > > * Renamed: > * -- EMPTY_STRING -> EMPTY (inherited) > * -- toUpperCase -> upperCase (inherited) > * -- toLowerCase -> lowerCase (inherited) > * -- padEnding -> rightPad (inherited) > * > * Removed: > * -- EMPTY_STRING_ARRAY; usage replaced with > commons.lang.ArrayUtils.EMPTY_STRING_ARRAY > * -- SPACES > * -- UTF_8; rare usage replaced with raw string > * -- concat; usage replaced with commons.lang.join, refactoring as > necessary. > * -- getLettersOnly > * -- getSpaces > * -- truncate > * -- valueOf; usage refactored to use defaultString > * > * Refactored > * -- defaultIfBlank: previously relied on varargs and could return null. > Usage refactored to allow inheritance from commons. > * -- defaultString(s, EMPTY) refactored to use standard signature > defaultString(s) for consistency throughout codebase. > * -- isBlank: usage refactored to resolve discrepancies with > commons.lang.isBlank, which is now inherited. > * -- isEmpty: usage refactored to resolve discrepancies with > commons.lang.isEmpty, which is now inherited. > * > * Code Cleanup: > * -- Many uses of !isBlank -> isNotBlank > * -- Changes suggested by Inspections on most touched files. > * -- Explicit <T> -> <> when type is inferable > * -- while loops operating on iterators converted to for each loops > * -- for loops operating on array indices converted to for each loops > * -- Various string typos corrected. > * -- isEmpty(s.trim()) -> isBlank(s) > * -- s.trim().isEmpty() -> isEmpty(s) > * -- Removed some instances of 'dead' code > * > * Qualitative Changes: > * -- The following functions now throw an error when called with a null > string input: > * -- * LocatorLauncher.Builder.setMemberName > * -- * ServerLauncher.Builder.setMemberName > * -- * ServerLauncher.Builder.setHostnameForClients > * > * Notes: > * -- StringUtils.wraps may be inherited from Apache Commons when the > dependency is updated. > * -- AbstractLauncher.getMember has the documented behavior of returning > null when both MemberName and ID are blank. Is this the best behavior for > this method? > > ====== > > I'm going through the diff myself now and noticing a lot of refactorings that > happen in dead code. I'm going back to cull the dead code entirely, but > wanted to go ahead and get this diff posted. > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java > 5277e57 > > geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsTest.java > 0554f69 > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPITestBase.java > f2e90a4 > > geode-core/src/main/java/org/apache/geode/cache/client/internal/locator/LocatorStatusResponse.java > d531cc1 > > geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java > f40ab3e > > geode-core/src/main/java/org/apache/geode/cache/query/internal/NWayMergeResults.java > 4e52a67 > geode-core/src/main/java/org/apache/geode/distributed/AbstractLauncher.java > feba893 > geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java > 641e009 > geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java > b2d0151 > > geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java > ad5e04d > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java > 4bf010b > > geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java > 78b2944 > > geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java > af3cb5d > > geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java > 55e3542 > geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java > 499d546 > geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java > 3485ede > > geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java > 629740c > > geode-core/src/main/java/org/apache/geode/internal/process/signal/Signal.java > faab4ed > > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java > 8366930 > geode-core/src/main/java/org/apache/geode/internal/util/ArrayUtils.java > 6f1c7cc > geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java > c1a1952 > > geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java > ba15508 > > geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java > 7c26297 > > geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java > 62310e8 > > geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java > 837e815 > > geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemBridge.java > ef643ac > > geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java > a87b366 > > geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java > 7dce602 > > geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java > f701d29 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java > 5dfc1b8 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java > 5b0651e > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/IndexCommands.java > 9110a1a > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java > 101bae4 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java > c9e79b0 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/GatewaySenderIdConverter.java > 59851da > > geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/IndexDetails.java > 64bb512 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java > 4383044 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java > 9dbf59c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ResultBuilder.java > 9bd2bf9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TableBuilder.java > da3c4ae > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java > 7c80e0d > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java > e9d183e > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java > 4410fea > > geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/XmlEntity.java > be74e84 > > geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java > 837d99e > > geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java > 3248c98 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/AbstractCommandsController.java > 0b64a44 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ConfigCommandsController.java > f468c65 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java > 3a8ed82 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DeployCommandsController.java > fe5be62 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DiskStoreCommandsController.java > 661340c > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DurableClientCommandsController.java > 1d718b4 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java > 604bdee > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/FunctionCommandsController.java > 77cfb40 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java > d19aee1 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/PdxCommandsController.java > c68ee35 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/QueueCommandsController.java > 396726e > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/RegionCommandsController.java > e503e56 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java > 0ecb77f > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/WanCommandsController.java > fa5aa57 > > geode-core/src/main/java/org/apache/geode/management/internal/web/domain/Link.java > cef8cab > > geode-core/src/main/java/org/apache/geode/management/internal/web/domain/LinkIndex.java > 7d5bb37 > > geode-core/src/main/java/org/apache/geode/management/internal/web/http/HttpHeader.java > ee7e932 > > geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java > eeedf40 > > geode-core/src/main/java/org/apache/geode/management/internal/web/util/UriUtils.java > b83064e > > geode-core/src/test/java/org/apache/geode/cache/client/internal/LocatorTestBase.java > c3b349a > > geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java > 62d4bdd > > geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java > 5b53c6a > > geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java > 06d6054 > > geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java > 7bd7462 > > geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java > f5d6271 > > geode-core/src/test/java/org/apache/geode/internal/cache/execute/Bug51193DUnitTest.java > 0dfbe6c > > geode-core/src/test/java/org/apache/geode/internal/lang/ClassUtilsJUnitTest.java > 3ec3b06 > > geode-core/src/test/java/org/apache/geode/internal/lang/StringUtilsJUnitTest.java > f3e0abe > > geode-core/src/test/java/org/apache/geode/internal/util/CollectionUtilsJUnitTest.java > 36be8b8 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupportJUnitTest.java > 91a59f8 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ListIndexCommandDUnitTest.java > 8bf1c43 > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java > fc920c4 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java > b2be12a > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java > d4dca4a > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/PropMockDataUpdater.java > 6ee5b53 > > geode-spark-connector/geode-functions/src/main/java/org/apache/geode/spark/connector/internal/geodefunctions/RetrieveRegionFunction.java > 096e4d5 > > geode-web/src/test/java/org/apache/geode/management/internal/web/AbstractWebTestCase.java > eac0b8d > > geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java > 37ec508 > > geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java > c69013b > > > Diff: https://reviews.apache.org/r/59323/diff/2/ > > > Testing > ------- > > Precheckin running. Tests and LegacyTests already returned green. > > > Thanks, > > Patrick Rhomberg > >
