> On Sept. 20, 2014, 2:14 a.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, > > line 286 > > <https://reviews.apache.org/r/25857/diff/2/?file=698339#file698339line286> > > > > I see why you went this route, but it's a precedent i'd rather not set. > > How about a guice-bound `@EnableUpdater Boolean`. Then, create a new test > > class that does nothing but create necessary mocks to construct an instance > > of SchedulerThriftInterface and test the flag. > > Maxim Khutornenko wrote: > This will result in a much larger diff for something that is only used > temporarily and will go away once all updater kinks are worked out. I'd > rather go with a smaller diff and document it as testing anti-pattern not to > be used for anything permanent.
A fair argument, but i'm still a big anti-fan of using reflection like this. I'd much rather the field be public than have false encapsulation. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25857/#review54071 ----------------------------------------------------------- On Sept. 19, 2014, 11:10 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25857/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2014, 11:10 p.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-732 > https://issues.apache.org/jira/browse/AURORA-732 > > > Repository: aurora > > > Description > ------- > > Added command line arg to control startJobUpdate. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 83ac034cff009530e5e16c0613b9d085f3b908d8 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 336cada625b85618486660fc24f3e83a312609f8 > > Diff: https://reviews.apache.org/r/25857/diff/ > > > Testing > ------- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
