Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-25 Thread Patrick Rhomberg
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review172975 --- Ship it! Ship It! - Patrick Rhomberg On April 13, 2017,

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-17 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review172124 --- Ship it! Ship It! - Kirk Lund On April 13, 2017, 10:13

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Jinmei Liao
> On April 13, 2017, 9:35 p.m., Kirk Lund wrote: > > geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java > > Line 58 (original), 58 (patched) > > > > > > I liked your

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171938 ---

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Kirk Lund
> On April 13, 2017, 7:25 p.m., Kirk Lund wrote: > > I'm finding lots of instances where "withRegion" was changed to > > "createRegion" but it's an API or Util class that isn't related to your > > Rule changes. You probably don't want to change all of these instances of > > "withRegion" --

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Jinmei Liao
> On April 13, 2017, 7:25 p.m., Kirk Lund wrote: > > geode-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java > > Line 77 (original), 77 (patched) > > > > > > Did you mean to make

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Jinmei Liao
> On April 13, 2017, 7:25 p.m., Kirk Lund wrote: > > I'm finding lots of instances where "withRegion" was changed to > > "createRegion" but it's an API or Util class that isn't related to your > > Rule changes. You probably don't want to change all of these instances of > > "withRegion" --

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171919 --- I'm finding lots of instances where "withRegion" was changed to

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Jinmei Liao
> On April 13, 2017, 5:08 p.m., Ken Howe wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > > Line 110 (original), 110 (patched) > > > > > > Rename withAutoStart() ?

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-13 Thread Ken Howe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171900 --- Fix it, then Ship it! Ship It!

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Jinmei Liao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/ --- (Updated April 12, 2017, 11:32 p.m.) Review request for geode, Jared Stewart,

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Ken Howe
> On April 12, 2017, 9:10 p.m., Ken Howe wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > > Lines 110 (patched) > > > > > > I find this method name confusing because the

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Jared Stewart
> On April 12, 2017, 9:10 p.m., Ken Howe wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > > Lines 110 (patched) > > > > > > I find this method name confusing because the

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Jinmei Liao
> On April 12, 2017, 9:10 p.m., Ken Howe wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > > Lines 110 (patched) > > > > > > I find this method name confusing because the

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Ken Howe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171788 --- Reviewed the rest of the diffs

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Ken Howe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171741 --- Reviewed the first page of diffs

Re: Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Jinmei Liao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/ --- (Updated April 12, 2017, 3:08 p.m.) Review request for geode, Jared Stewart,

Review Request 58388: GEODE-2730: refactor rules

2017-04-12 Thread Jinmei Liao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/ --- Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick