> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, 
> > lines 296-305
> > <https://reviews.apache.org/r/25963/diff/1/?file=703457#file703457line296>
> >
> >     Can we just have a single function that takes the statuses as a 
> > parameter? If you want to maintain the simplicity of the current API we 
> > could bind a more generic function to a specific one. I.e.:
> >     
> >     function getJobUpdateQuery(status) {
> >       var query = new JobUpdateQuery();
> >       query.updateStatuses = status;
> >       return query;
> >     };
> >     
> >     ...
> >     
> >     getTerminalQuery: getJobUpdateQuery.bind(this, UPDATE_TERMINAL_STATUSES 
> > );

The API is so that services.js was solely responsible for what constitutes a 
TERMINAL_STATE, rather than leaking that code into the controllers. There are 
multiple ways to do that, of course. For now I will make the refactor you 
proposed.


> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, 
> > line 39
> > <https://reviews.apache.org/r/25963/diff/1/?file=703456#file703456line39>
> >
> >     restore the `_MS` suffix to make it explicit what the unit is here?

Fixed.


> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html, 
> > lines 33-51
> > <https://reviews.apache.org/r/25963/diff/1/?file=703458#file703458line33>
> >
> >     This table and the one below for completed updates are identical (minus 
> > the data source). Does angular have partials or something similar so we can 
> > avoid replicating this?

Sure, I can make this a directive. I figured we might eventually have 
pagination/filtering/etc. in the second table but not the first, so that was 
the reasoning.. but we can cross that bridge when we come to it.


- David


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


On Sept. 23, 2014, 11:46 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 11:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
>     https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> ----------------
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Reply via email to