> On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > Can you see if the test coverage increase/decrease after this change
The change is minimal, if any. There are a few more error checks, leading to a slight decrease. > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java, > > line 44 > > <https://reviews.apache.org/r/13390/diff/2/?file=338469#file338469line44> > > > > remove this line, Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java, > > line 42 > > <https://reviews.apache.org/r/13390/diff/2/?file=338471#file338471line42> > > > > remove this line Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java, > > line 46 > > <https://reviews.apache.org/r/13390/diff/2/?file=338474#file338474line46> > > > > javadoc Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java, > > line 37 > > <https://reviews.apache.org/r/13390/diff/2/?file=338471#file338471line37> > > > > java doc Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java, > > line 38 > > <https://reviews.apache.org/r/13390/diff/2/?file=338469#file338469line38> > > > > javadoc for this, give an example input/output Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java, > > line 68 > > <https://reviews.apache.org/r/13390/diff/2/?file=338470#file338470line68> > > > > Having a map<string,map<String,string> is confusing. Have a new class > > called ResourceMapping or something like that Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/stages/util/BestStateCalculator.java, > > line 1 > > <https://reviews.apache.org/r/13390/diff/2/?file=338473#file338473line1> > > > > should probably be under rebalancer.util. This is kind of the > > constraint solver right. need a better name. Done > On Aug. 7, 2013, 11:33 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java, > > line 219 > > <https://reviews.apache.org/r/13390/diff/2/?file=338472#file338472line219> > > > > see the OSGi patch. Using class.forname is not liked by OSGi Done - Kanak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13390/#review24847 ----------------------------------------------------------- On Aug. 7, 2013, 11:26 p.m., Kanak Biscuitwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13390/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2013, 11:26 p.m.) > > > Review request for helix, Zhen Zhang, Kishore Gopalakrishna, and Shi Lu. > > > Bugs: HELIX-173 > > > Repository: helix-git > > > Description > ------- > > Fix for HELIX-173. Right now, the code to compute mappings from partitions to > nodes and states is mostly in a single file, regardless of the rebalancing > mode. Moving these operations cleans up the code and is more logical for > pluggable rebalancers. > > > Diffs > ----- > > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/Rebalancer.java > 62a73b3 > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java > aca0e74 > > helix-core/src/main/java/org/apache/helix/controller/stages/util/BestStateCalculator.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java > f013937 > > helix-core/src/test/java/org/apache/helix/controller/strategy/TestAutoRebalanceStrategy.java > 50a2f81 > > helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java > df32c4e > > Diff: https://reviews.apache.org/r/13390/diff/ > > > Testing > ------- > > Tests pass locally. > > > Thanks, > > Kanak Biscuitwala > >
