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


Fix it, then Ship it!





src/webui/app/controllers.js
Lines 714-717 (patched)
<https://reviews.apache.org/r/69662/#comment297385>

    Is this something we do elsewhere with IDs? Is it because elsewhere we 
don't have value in the json? Feels unfortunate to have to mutate the structure 
of the json (I know we have done that in a few places but this seems like a 
mutation of structure rather than adding things)
    
    Maybe at the very least if we do think we should do this here, we can add 
some context about it (e.g. json has value for this id but not others)



src/webui/app/controllers.js
Lines 722-723 (patched)
<https://reviews.apache.org/r/69662/#comment297386>

    This seems not very readable (or at least I struggled a bit with it), can 
we use underscore js clone?
    
    https://underscorejs.org/#clone
    
    ```
    provider.total_resources_full = _.clone(provider.total_resources);
    ```


- Benjamin Mahler


On Jan. 7, 2019, 10:33 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69662/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2019, 10:33 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, and Chun-Hung 
> Hsiao.
> 
> 
> Bugs: MESOS-8380
>     https://issues.apache.org/jira/browse/MESOS-8380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Displayed resource provider information in the Mesos webui.
> 
> 
> Diffs
> -----
> 
>   src/webui/app/agents/agent.html a101a93dcdb95f257fe0ee967c92d2cdc1c84f84 
>   src/webui/app/controllers.js 8049cf611895edea7c54b3c58d71e00d823a1fd3 
> 
> 
> Diff: https://reviews.apache.org/r/69662/diff/4/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> Ran a local test with a `./src/test-csi-plugin`.
> 
> 
> File Attachments
> ----------------
> 
> Screenshot Agent screen
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/04/ed920e7b-4072-49be-8801-3b875d529fad__Screen_Shot_2019-01-04_at_11.11.51_AM.png
> Screenshot Agent screen
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/07/8f494c0f-1c76-4734-9aae-7fb899589120__Screen_Shot_2019-01-07_at_9.42.51_PM.png
> Screenshot Agent screen
>   
> https://reviews.apache.org/media/uploaded/files/2019/01/07/8a850bcd-dd30-4d25-bb37-60cd872ddd62__Screen_Shot_2019-01-07_at_11.27.49_PM.png
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Reply via email to