----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59323/#review175278 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java Line 29 (original), 29 (patched) <https://reviews.apache.org/r/59323/#comment248722> I would not make this class extends apache common's StringUtils. We should always encourage devleopers to use the apache's String utils, and only for very specific cases, we would want to use our own. - Jinmei Liao 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 > >
