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


Ship it!





src/common/parse.hpp (line 123)
<https://reviews.apache.org/r/44777/#comment188857>

    Do we need this temporary?
    
    ```
    foreach (const std::string& token, strings::tokenize(value, ",")) {
    ```



src/common/parse.hpp (line 128)
<https://reviews.apache.org/r/44777/#comment188856>

    Unfortunately numify doesn't follow our error composition approach of the 
callee not repeating information available to the caller, so if we followed our 
composition pattern here we would get:
    
    ```
    return Error("Failed to numify token '" + token + "'"
                 ": " + numify.error());
    
    // because numify is repeating caller-available
    // information, this yields:
    //
    // "Failed to numify 'a': Failed to convert 'a' to number"
    ```
    
    But let's still do this for now, so that this continue to be meaningful 
once we update numify logging to not repeat the arguments available in the 
caller.


- Ben Mahler


On March 18, 2016, 12:14 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44777/
> -----------------------------------------------------------
> 
> (Updated March 18, 2016, 12:14 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4926
>     https://issues.apache.org/jira/browse/MESOS-4926
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a flags parser for vector<unsigned int> to src/common/parse.hpp.
> 
> 
> Diffs
> -----
> 
>   src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 
> 
> Diff: https://reviews.apache.org/r/44777/diff/
> 
> 
> Testing
> -------
> 
> Ran `make` and `make check`
> 
> This is also tested out in a subsequent commit when adding support for the 
> `--nvidia_gpu_devices` flag, which uses this functionality.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Reply via email to