Re: Review Request 19833: Migrated Job page to angular JS

2014-04-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review40748 --- Ship it! My only concern is I think the controller code can be

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review40749 --- Ship it! My only concern is I think the controller code can be

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-18 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 19, 2014, 2:04 a.m.) Review request for Aurora, David

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-17 Thread Suman Karumuri
On April 17, 2014, 12:21 a.m., David McLaughlin wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, lines 361-363 https://reviews.apache.org/r/19833/diff/7/?file=560830#file560830line361 Better to move these default values to getTasksForJob (is the

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-17 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 18, 2014, 1:59 a.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-17 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 18, 2014, 2 a.m.) Review request for Aurora, David McLaughlin

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-17 Thread David McLaughlin
On April 17, 2014, 12:21 a.m., David McLaughlin wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 365 https://reviews.apache.org/r/19833/diff/7/?file=560830#file560830line365 Why are you passing $scope.role, $scope.environment and $scope.job

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread Suman Karumuri
On April 15, 2014, 8:15 p.m., Kevin Sweeney wrote: A few general Javascript comments: there seems to have been an explosion of global state and some controllers are getting pretty big. Consider using angular's dependency injection system to keep dependencies explicit and managed

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread Suman Karumuri
On April 6, 2014, 12:24 a.m., Bill Farner wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/schedulingDetail.html, line 9 https://reviews.apache.org/r/19833/diff/6/?file=549479#file549479line9 remove space before the colon, here and below Done. On April 6, 2014,

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread David McLaughlin
On April 6, 2014, 12:24 a.m., Bill Farner wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/job.html, line 62 https://reviews.apache.org/r/19833/diff/6/?file=549472#file549472line62 Can you make this an anchor instead of span? Also, the issue i pointed out

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread Suman Karumuri
On April 16, 2014, 12:18 a.m., David McLaughlin wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/job.html, line 30 https://reviews.apache.org/r/19833/diff/6/?file=549472#file549472line30 You don't need noMarginLeft here. The left padding is only applied to .span*

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 16, 2014, 8:37 a.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 16, 2014, 8:37 a.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review40608 --- src/main/resources/org/apache/aurora/scheduler/http/ui/job.html

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-15 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review40443 --- A few general Javascript comments: there seems to have been an

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-09 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review39908 --- pinging Kevin. - Suman Karumuri On April 4, 2014, 11:52 p.m.,

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-09 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review39909 --- pinging Kevin. - Suman Karumuri On April 4, 2014, 11:52 p.m.,

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review39642 --- Ship it! Overall LGTM. After you get a ship from Kevin, please be

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-02 Thread Suman Karumuri
On April 1, 2014, 10:55 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 134 https://reviews.apache.org/r/19833/diff/4/?file=544786#file544786line134 Can you address this TODO now? AFAICT there's nothing in the way of that. This

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-02 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 2, 2014, 11:26 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-01 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated April 1, 2014, 9:57 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 19833: Migrated Job page to angular JS

2014-04-01 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/#review39208 --- Looks great overall! Last request - can you push a branch to

Re: Review Request 19833: Migrated Job page to angular JS

2014-03-31 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- (Updated March 31, 2014, 6:09 a.m.) Review request for Aurora, Kevin Sweeney

Review Request 19833: Migrated Job page to angular JS

2014-03-30 Thread Suman Karumuri
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19833/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora