Review Request 21754: Add PMD to the build.

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21754/ --- Review request for Aurora, David McLaughlin, Kevin Sweeney, and Maxim

Review Request 21758: Upgrade to gradle 1.12.

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21758/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description

Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

2014-05-21 Thread Mark Chu-Carroll
On May 20, 2014, 10:58 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java, line 135 https://reviews.apache.org/r/21383/diff/4-5/?file=584804#file584804line135 remove trailing WS. this should be trivial to catch in

Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

2014-05-21 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21383/ --- (Updated May 21, 2014, 9:05 a.m.) Review request for Aurora, David McLaughlin

Re: Review Request 21754: Add PMD to the build.

2014-05-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21754/#review43617 --- Does it bring anything else besides parentheses checks that

Re: Review Request 21754: Add PMD to the build.

2014-05-21 Thread Bill Farner
On May 21, 2014, 3:01 p.m., Maxim Khutornenko wrote: Does it bring anything else besides parentheses checks that findbugs does not already do? My only concern is build performance. That said, if it does not take long to run I am in.

Re: Review Request 21383: Add cron schedule and deschedule calls to the scheduler API.

2014-05-21 Thread Bill Farner
On May 21, 2014, 2:58 a.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java, line 135 https://reviews.apache.org/r/21383/diff/4-5/?file=584804#file584804line135 remove trailing WS. this should be trivial to catch in

Re: Review Request 21754: Add PMD to the build.

2014-05-21 Thread Bill Farner
On May 21, 2014, 3:01 p.m., Maxim Khutornenko wrote: Does it bring anything else besides parentheses checks that findbugs does not already do? My only concern is build performance. That said, if it does not take long to run I am in. Bill Farner wrote:

Re: Review Request 21597: Return empty list instead of failing.

2014-05-21 Thread Maxim Khutornenko
On May 21, 2014, 1:38 a.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/base.py, line 47 https://reviews.apache.org/r/21597/diff/2/?file=584355#file584355line47 Why are these split into multiple methods? The only place where check_and_log_locked is used is right

Re: Review Request 20285: Improve documentation and testing for host maintenance API

2014-05-21 Thread Tobias Weingartner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20285/#review43628 --- Ship it! The API seems a bit strange to me here. In particular,

Re: Review Request 21597: Return empty list instead of failing.

2014-05-21 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21597/#review43634 --- Ship it! Ship It! - Mark Chu-Carroll On May 21, 2014, 12:05

Re: Review Request 21440: Implementing parallel updater

2014-05-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/ --- (Updated May 21, 2014, 4:58 p.m.) Review request for Aurora, Mark Chu-Carroll

Re: Review Request 21440: Implementing parallel updater

2014-05-21 Thread Maxim Khutornenko
On May 20, 2014, 8:04 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/updater.py, line 58 https://reviews.apache.org/r/21440/diff/2/?file=582780#file582780line58 This could be clearer with a bit of rephrasing in the comment: A thread that than

Review Request 21780: Adding cron client commands.

2014-05-21 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21780/ --- Review request for Aurora, David McLaughlin and Suman Karumuri. Bugs:

Review Request 21781: Removing --groups_per_batch option.

2014-05-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21781/ --- Review request for Aurora, Joe Smith and Brian Wickman. Bugs: AURORA-446

Re: Review Request 21753: Enable a few checkstyle checks.

2014-05-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21753/#review43644 --- Ship it! - David McLaughlin On May 21, 2014, 5:13 a.m., Bill

Re: Review Request 21754: Add PMD to the build.

2014-05-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21754/#review43646 --- Ship it! Ship It! - David McLaughlin On May 21, 2014, 6:05

Re: Review Request 21780: Adding cron client commands.

2014-05-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21780/#review43647 --- Ship it! Ship It! - David McLaughlin On May 21, 2014, 5:45

Re: Review Request 21132: Initial attempt at h2/DB storage implementation (LockStore only)

2014-05-21 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21132/#review43377 --- Ship it!

Re: Review Request 21740: Use rsync instead of git to copy code into vagrant, simplify end-to-end tests.

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21740/ --- (Updated May 21, 2014, 7:50 p.m.) Review request for Aurora, Kevin Sweeney and

Re: Review Request 21758: Upgrade to gradle 1.12.

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21758/ --- (Updated May 21, 2014, 8:07 p.m.) Review request for Aurora and Maxim

Re: Review Request 21740: Use rsync instead of git to copy code into vagrant, simplify end-to-end tests.

2014-05-21 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21740/#review43653 --- Ship it! Ship It! - Kevin Sweeney On May 21, 2014, 12:50 p.m.,

Re: Review Request 21739: Add a test to catch regressions in scheduler_client connect_scheduler.

2014-05-21 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21739/#review43656 --- Ship it! Ship It! - Kevin Sweeney On May 20, 2014, 4:57 p.m.,

Re: Review Request 21739: Add a test to catch regressions in scheduler_client connect_scheduler.

2014-05-21 Thread Kevin Sweeney
On May 20, 2014, 6:34 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, line 87 https://reviews.apache.org/r/21739/diff/1/?file=585799#file585799line87 Not necessary, but you could just use a patch in the test, instead of changing this.

Re: Review Request 21753: Enable a few checkstyle checks.

2014-05-21 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21753/#review43657 --- Ship it! Ship It! - Maxim Khutornenko On May 21, 2014, 8:02

Re: Review Request 21753: Enable a few checkstyle checks.

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21753/ --- (Updated May 21, 2014, 9:54 p.m.) Review request for Aurora, David McLaughlin

Review Request 21791: Do not show instance range when there is only one in group

2014-05-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21791/ --- Review request for Aurora, Suman Karumuri and Mark Chu-Carroll. Bugs:

Re: Review Request 21791: Do not show instance range when there is only one in group

2014-05-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21791/#review43664 --- I agree with the wording on the ticket more than what's done here.

Re: Review Request 21791: Do not show instance range when there is only one in group

2014-05-21 Thread David McLaughlin
On May 21, 2014, 10:42 p.m., Bill Farner wrote: I agree with the wording on the ticket more than what's done here. Having the range displayed when there's one group is still useful to see the instance count quickly. It also verifies that there are not instances 'missing' (e.g. 4-99

Re: Review Request 21523: Make JS compliant with JSHint rules

2014-05-21 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21523/#review43671 --- Pushed. Please close out this review. - Suman Karumuri On May

Re: Review Request 21739: Add a test to catch regressions in scheduler_client connect_scheduler.

2014-05-21 Thread Joe Smith
On May 20, 2014, 6:34 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, line 87 https://reviews.apache.org/r/21739/diff/1/?file=585799#file585799line87 Not necessary, but you could just use a patch in the test, instead of changing this.