On Feb. 14, 2017, 4:33 p.m., Kirk Lund wrote: > > Everything else looks great.
It happens from following TDD. I write the test first and then write the code. This is an example where the results of TDD produce a test with questionable value, but if the tests are very fast (and they are) then it's better to have more tests than fewer. In particular, our project has a history of bad code and too few unit tests, so it's a good habit to follow for Geode. - Kirk ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56633/#review165524 ----------------------------------------------------------- On Feb. 14, 2017, 2:31 a.m., Kirk Lund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56633/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2017, 2:31 a.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Kevin Duling, and Ken > Howe. > > > Bugs: GEODE-2474 > https://issues.apache.org/jira/browse/GEODE-2474 > > > Repository: geode > > > Description > ------- > > GEODE-2474: refactor code to use SystemUtils to read OS system props > > Centralize OS system property reading in SystemUtils. > > Refactor NetstatFunction and GemFireVersion to use SystemUtils. > > This fixes use of --with-lsof on Mac (manually tested). I'll add new tests to > NetstatDUnitTest and a new integration test for netstat command in a > follow-up commit & review. > > I have several changes on feature/GEODE-2474 branch which I'll separate into > multiple reviews. I'll do a final precheckin on the entire branch and then > merge the changes in after everything passes review and precheckin. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/GemFireVersion.java > 26d4fb3c5705bffdcdbbc6c261dbe9ffd297642e > geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java > 66c158c93fecac4feb2da56f617f5efc7bba56e1 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/NetstatFunction.java > 5fa30f47187972a209a781eae9024957dc80fb72 > > geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java > 48f176eabc18d3ffa56daaa7da12634a9554f39d > > Diff: https://reviews.apache.org/r/56633/diff/ > > > Testing > ------- > > SystemUtilsJUnitTest > NetstatDUnitTest > > > Thanks, > > Kirk Lund > >
