> On March 11, 2014, 12:33 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/base/Tasks.java, line 51 > > <https://reviews.apache.org/r/17303/diff/4/?file=514590#file514590line51> > > > > looks like this just barely fits on the previous line
Done. > On March 11, 2014, 12:33 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/base/Tasks.java, line 223 > > <https://reviews.apache.org/r/17303/diff/4/?file=514590#file514590line223> > > > > Ordering.max already throws NoSuchElementException, which is fine. > > > > However, you should generally prefer to use Preconditions.checkArgument > > if you were to perform the check here. Preferring an explicit check to make intent clear. Changed to use Preconditions.checkArgument. > On March 11, 2014, 12:33 a.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java, line 37 > > <https://reviews.apache.org/r/17303/diff/4/?file=514596#file514596line37> > > > > OrderedTaskStatuses seems to refer to a thing, but i can't find the > > thing. Should this comment be rewritten? Oh that was a typo. SchedulerStatus -> ScheduleStatus > On March 11, 2014, 12:33 a.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java, line 57 > > <https://reviews.apache.org/r/17303/diff/4/?file=514596#file514596line57> > > > > Always default to immutable. > > > > ImmutableList.<IScheduledTask>of() > > > > Changed. - Suman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/#review36735 ----------------------------------------------------------- On March 9, 2014, 10:17 a.m., Suman Karumuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17303/ > ----------------------------------------------------------- > > (Updated March 9, 2014, 10:17 a.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-64 > https://issues.apache.org/jira/browse/AURORA-64 > > > Repository: aurora > > > Description > ------- > > Added getJobSummary API so it can be used by the role and role/environment > page in the UI. > Refactored code from SchedulerzRole and SchedulerzRoleTest into relevant > classes so it can be used by the UI and the thrift API. > Added tests for new code. > Moved populateJobConfig call into ReadOnlyScheduler. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/base/Jobs.java PRE-CREATION > src/main/java/org/apache/aurora/scheduler/base/Tasks.java > d9cb886ef333e108d5d5f86043ac80e450689894 > src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java > 60b2259f21b598fa38bec5a590516cba2c07e1ac > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 4911c77a16c486fabfead7ad2f84ee95423ecd93 > src/main/thrift/org/apache/aurora/gen/api.thrift > d72b28c3378a651a8cff49216c1435ce7aee5977 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/http/SchedulerzRoleTest.java > 912be189583419e7201e45650d18cd24a6a5a35b > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 9712f30a71be206fbf417198d0af673b45e0281e > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java > a5fcbd465b5e07e23b24524e060cea304f102492 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 2308ba8da96197d41040ba772ea871003615698a > > Diff: https://reviews.apache.org/r/17303/diff/ > > > Testing > ------- > > gradle clean build > gradle run to test local UI. > > > Thanks, > > Suman Karumuri > >
