> On June 6, 2017, 11:17 a.m., Eric Shu wrote: > > geode-core/src/main/java/org/apache/geode/cache/partition/StringPrefixPartitionResolver.java > > Lines 60 (patched) > > <https://reviews.apache.org/r/59849/diff/1/?file=1742650#file1742650line60> > > > > Does getName() needs the Delimiter being included? If it is used in the > > equals(), can we just check getDelimiter() in the equals method?
I thought it did but I reviewed how the product uses getName and it seems like it expects it to be just the class name of the resolver. So I will change getName to not include it and change equals and hashCode to also call getDelimiter. Thanks for the review. - Darrel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59849/#review177064 ----------------------------------------------------------- On June 6, 2017, 10:44 a.m., Darrel Schneider wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59849/ > ----------------------------------------------------------- > > (Updated June 6, 2017, 10:44 a.m.) > > > Review request for geode, Eric Shu and Lynn Gallinat. > > > Bugs: GEODE-3027 > https://issues.apache.org/jira/browse/GEODE-3027 > > > Repository: geode > > > Description > ------- > > The new resolver uses a fixed delimiter of "|". > In this version of the resolver you can not configure the delimiter. > "|" was chosen over ":" because it seemed less likely that the desired prefix > string would contain it. > Since the delimiter can not be configured it is important that it would not > be a common one to occur in a key string. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/cache/partition/StringPrefixPartitionResolver.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/cache/partition/StringPrefixPartitionResolverJUnitTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/59849/diff/1/ > > > Testing > ------- > > precheckin > > > Thanks, > > Darrel Schneider > >
