> On July 21, 2014, 10:16 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 721 > > <https://reviews.apache.org/r/23254/diff/1/?file=623290#file623290line721> > > > > We should avoid taking advantage of passing nulls where possible, and > > in fact i suggest changing addMessage to MorePreconditions.checkNotBlank on > > that. However, you'll need to take care in cases like addMessage(x, x, > > exception.getMessage()). For those, it makes sense to add an overload that > > accepts an Exception instead of String.
Makes sense. Done. > On July 21, 2014, 10:16 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java, > > line 1 > > <https://reviews.apache.org/r/23254/diff/1/?file=623294#file623294line1> > > > > Is this file relevant to the change? This is related to the "Also moving some useful tests out of BaseSchedulerCoreImplTest." part of the changelist. Trying to balance the diff size. > On July 21, 2014, 10:16 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, > > line 431 > > <https://reviews.apache.org/r/23254/diff/1/?file=623295#file623295line431> > > > > There's a subtle detail in this test case, that active() is > > automatically added to the task query. Can you break that out into an > > independent test case? Sure, done. > On July 21, 2014, 10:16 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java, > > line 509 > > <https://reviews.apache.org/r/23254/diff/1/?file=623295#file623295line509> > > > > Use a constant for string that is otherwise magic. Done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23254/#review48302 ----------------------------------------------------------- On July 3, 2014, 1:48 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23254/ > ----------------------------------------------------------- > > (Updated July 3, 2014, 1:48 a.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-94 > https://issues.apache.org/jira/browse/AURORA-94 > > > Repository: aurora > > > Description > ------- > > Splitting the earlier RB for easier reviewing. > > Addressing killTasks refactoring. Also moving some useful tests out of > BaseSchedulerCoreImplTest. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/http/ServletModule.java > 27599f75603542069084631baf9195b8ad75e902 > src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java > 628464de0032db4eb5884e3b74346b2390408993 > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java > b6d06f39ac39570eac4495d97d3adaabe8357bf7 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 2549dd33d08dfc6058d985127a3f0c1f3984eaa7 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 2298971ffac45a284f9130e2122aeea8b39dc852 > > src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java > d2bd65eef5a8ceea92dd62044e5e2c621c4a4f3e > src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java > 94eb0a22037187625636b24707d4ac6741b55f22 > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 2cffa74ba36e2afda3340658d6b1afd6cb50cf2c > > Diff: https://reviews.apache.org/r/23254/diff/ > > > Testing > ------- > > gradle -Pq clean build > > > Thanks, > > Maxim Khutornenko > >
