----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59323/#review175428 -----------------------------------------------------------
Ship it! Ship It! - Kirk Lund On May 18, 2017, midnight, Patrick Rhomberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59323/ > ----------------------------------------------------------- > > (Updated May 18, 2017, midnight) > > > Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, > and Swapnil Bawaskar. > > > Repository: geode > > > Description > ------- > > * geode.internal.lang.StringUtils has been deprecated. In the interim, it > has been heavily refactored and extends commons.lang.StringUtils. > * > * 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 > * -- (Unit tests added to capture these changes) > * > * 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/ClusterConfigurationService.java > 10623b4 > > 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/cache/EntryEventImpl.java > 185fde7 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java > 0b11bf1 > 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 > 2e47556 > > 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/SSLUtil.java > 1b39b73 > > 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/CliUtil.java > bd6d810 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AbstractCommandsSupport.java > ae44e24 > > 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/ExportImportClusterConfigurationCommands.java > dfd20a9 > > 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/FilePathStringConverter.java > 3fadd96 > > 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/DiskStoreDetails.java > b5d1b0c > > 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/functions/DescribeDiskStoreFunction.java > 4c241d0 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/FetchSharedConfigurationStatusFunction.java > 57d209b > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java > c99f84a > > 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/GfshConfig.java > c35f420 > > 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 > 0dbe7e5 > > 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/configuration/utils/XmlUtils.java > 218762c > > 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/ConvertUtils.java > f3b092d > > 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/AbstractLauncherIntegrationTestCase.java > 09fa09e > > 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/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-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > 19a1662 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java > b2be12a > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java > f7edd8f > > 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/3/ > > > Testing > ------- > > Precheckin running. Tests and LegacyTests already returned green. > > > Thanks, > > Patrick Rhomberg > >