> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/noop/NoopCronPredictor.java, 
> > line 28
> > <https://reviews.apache.org/r/19450/diff/1/?file=529215#file529215line28>
> >
> >     revert

Changed.


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 420
> > <https://reviews.apache.org/r/19450/diff/1/?file=529216#file529216line420>
> >
> >     no need to abbreviate here

Abbreviated for better formatting and readability of the conditional expression.


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java,
> >  line 80
> > <https://reviews.apache.org/r/19450/diff/1/?file=529218#file529218line80>
> >
> >     Please use a mock here.

Changed


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java,
> >  line 194
> > <https://reviews.apache.org/r/19450/diff/1/?file=529218#file529218line194>
> >
> >     +1 to mock.

Changed


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java,
> >  line 984
> > <https://reviews.apache.org/r/19450/diff/1/?file=529218#file529218line984>
> >
> >     Assert a mocked value here instead.

Changed.


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java, line 39
> > <https://reviews.apache.org/r/19450/diff/1/?file=529219#file529219line39>
> >
> >     Mock here. In general prefer to avoid importing production classes 
> > across packages in unit tests

Didn't mock since NoopCronPredictor was kind of mock. Changed it now.


> On March 20, 2014, 6:16 p.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java, line 169
> > <https://reviews.apache.org/r/19450/diff/1/?file=529219#file529219line169>
> >
> >     +1 to mock

Changed.


- Suman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19450/#review37929
-----------------------------------------------------------


On March 20, 2014, 10:22 p.m., Suman Karumuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19450/
> -----------------------------------------------------------
> 
> (Updated March 20, 2014, 10:22 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-277
>     https://issues.apache.org/jira/browse/AURORA-277
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added nextCronRunMs field to JobSummary.
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  0d1e2147ce6a370962dab93383d52ddeba68efb5 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> e64f844cbd58d92621c6c21b896a9baf0c5a5f07 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  60df209150f29e2658c47d81042bd36c57d5afef 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> e78ee7b8051bc080dbb3ed2daf72171bffe3915a 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> 456634c76b9993119af2d841073c31a4a1c82ab7 
> 
> Diff: https://reviews.apache.org/r/19450/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew clean build run on laptop.
> 
> 
> Thanks,
> 
> Suman Karumuri
> 
>

Reply via email to