> On Jan. 19, 2016, 4:40 p.m., Jinmei Liao wrote: > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/RegionVersionHolder.java, > > line 331 > > <https://reviews.apache.org/r/42465/diff/2/?file=1200643#file1200643line331> > > > > Looks like in the recordVersionWithoutBitSet, in case when version == > > this.version and version= this.version + 1, we are still calling > > this.addOlderVersion(version). Would this have any unexpected impact on the > > end result? > > Bruce Schuchardt wrote: > I think that's intentional, for the "special exception" - though I'm > still a bit unclear about what this exception is supposed to be. That's what > the old code was doing, in any case, and the refactoring is supposed to > preserve the old behavior.
Oh, by looking more closely to the old behavior, in the case of "version==this.version" it is calling addOlderVersion, but in the case (version == this.version+1), it does not look like it's doing that......., or maybe I am wrong. Just so that you can take a look. Thanks! - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42465/#review115180 ----------------------------------------------------------- On Jan. 18, 2016, 6:29 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42465/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2016, 6:29 p.m.) > > > Review request for geode, Dan Smith and xiaojian zhou. > > > Repository: geode > > > Description > ------- > > This is from a group refactoring session during a class taught by Michael > Feathers. The old recordVersion code was difficult to understand and was > poorly tested. New JUnit tests are now included that give 100% code coverage > for this algorithm. > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/RegionVersionHolder.java > 4872df2a06c0bf6c949204cf98074d9259d6413f > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/RegionVersionHolder2JUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/42465/diff/ > > > Testing > ------- > > precheckin including new tests > > > Thanks, > > Bruce Schuchardt > >
