----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16220/#review30316 -----------------------------------------------------------
Looking much better, i suspect this is the last round-trip before a ship. src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java <https://reviews.apache.org/r/16220/#comment58025> At this point, you might as well just return Response from the storage op src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java <https://reviews.apache.org/r/16220/#comment58032> This is probably more appropriately-named getTasksByHosts() src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java <https://reviews.apache.org/r/16220/#comment58031> (1) please leave a blank line between a wrapped method signature and the method body (2) indentation is funky in this method src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java <https://reviews.apache.org/r/16220/#comment58027> I generally dislike embedding the type in the variable name. IMHO, 'attributes' is sufficient, and the type explains that it might be absent. src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java <https://reviews.apache.org/r/16220/#comment58029> I would find this more concise and more readable: if (attributes.isPresent() && (DRAINING == attributes.getMode())) { ... } src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java <https://reviews.apache.org/r/16220/#comment58026> inline below src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java <https://reviews.apache.org/r/16220/#comment58030> please kindly give intellij a smack and revert - Bill Farner On Dec. 13, 2013, 12:56 a.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16220/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2013, 12:56 a.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-9 > https://issues.apache.org/jira/browse/AURORA-9 > > > Repository: aurora > > > Description > ------- > > Improve the /maintenance endpoint to print out hosts affected by SCHEDULED > and DRAINED states. > > > Diffs > ----- > > src/main/java/com/twitter/aurora/scheduler/http/Maintenance.java > 30afce37d6c108a5a8c1c3c8a8094030ad12ce72 > src/main/java/com/twitter/aurora/scheduler/state/MaintenanceController.java > fb12d38858b260d1d9ce318d3022cd93413a3e68 > > src/test/java/com/twitter/aurora/scheduler/state/MaintenanceControllerImplTest.java > 8acb716c733ec9d3cc3b1ec74c85f958082ae139 > > Diff: https://reviews.apache.org/r/16220/diff/ > > > Testing > ------- > > ./gradlew clean build > > > Thanks, > > Zameer Manji > >
