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


I might want a loader, so the user knows that something is going on, or at 
least empty the current list.


src/webui/master/static/config.html
<https://reviews.apache.org/r/17431/#comment97367>

    Maybe a convinience method? Took me a while to figure out what was going on.



src/webui/master/static/js/app.js
<https://reviews.apache.org/r/17431/#comment97366>

    `;` after return statement?



src/webui/master/static/js/app.js
<https://reviews.apache.org/r/17431/#comment97365>

    `;` after return statement?



src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/17431/#comment97371>

    `=== -1`?
    
    Also it seems that you're doing something similar here to what you're doing 
in config.html:
    `location.absUrl().split('/').slice(0,3).join('/')`
    
    I think puttings these into convenience methods somewhere would be good.



src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/17431/#comment97363>

    `if (config.host === '')`?


I didn't get too much into overall linting errors that are not in your code, 
but there are some errors that could correct. However, I'm not sure that's a 
task for this patch.

- Michael Lunøe


On Oct. 14, 2014, 10:54 p.m., Thomas Rampelberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17431/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2014, 10:54 p.m.)
> 
> 
> Review request for mesos, Michael Lunøe and Niklas Nielsen.
> 
> 
> Bugs: mesos-885
>     https://issues.apache.org/jira/browse/mesos-885
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Enabled configuration of the mesos master from the UI.
> 
> Review: http://reviews.apache.org/r/17431
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
>   src/webui/master/static/config.html PRE-CREATION 
>   src/webui/master/static/css/mesos.css 
> 5b1227e9d64757f9fc106e497f7fa3ed72112c10 
>   src/webui/master/static/directives/timestamp.html 
> 5e422b9f22f8ddaf987feec3e02a849f21e5e22c 
>   src/webui/master/static/index.html 25caf530628ad3ac7f23ab5f014000aac8583da1 
>   src/webui/master/static/js/app.js 4ccff6314c684ae4e917345fe41a95ccc0eb5803 
>   src/webui/master/static/js/controllers.js 
> 41a70a80442501a2bf7b217939dbe504662941d2 
> 
> Diff: https://reviews.apache.org/r/17431/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Config Dialog
>   
> https://reviews.apache.org/media/uploaded/files/2014/01/28/5499d3e5-077e-4aff-b29a-7d32134f29a0__Screenshot_2014-01-27_16.53.12.png
> Connection Issue Alert
>   
> https://reviews.apache.org/media/uploaded/files/2014/01/28/dee8df12-0bae-48b5-a7ce-c07e0266c790__Screenshot_2014-01-28_12.44.53.png
> 
> 
> Thanks,
> 
> Thomas Rampelberg
> 
>

Reply via email to