Re: Review Request 50128: Added helper functions to 'Docker::Device'.

[Kevin Klues]

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




src/docker/docker.hpp (lines 73 - 84)
<https://reviews.apache.org/r/50128/#comment213390>

    In general, we don't typically have constructors for `structs` like this. 
Instead, we just set its fields directly at the call site.
    
    If you see the comment below about making the `Device::parse()` function 
static, then you could (in theory) always build the device from that instead of 
calling the constructor directly, i.e.:
    
    ```
    Try<Device> device = Device::parse(deviceString);
    ```
    
    Device device;
    device.hostPath = host;
    device.containerPath = container;
    device.access = ;
    ```



src/docker/docker.hpp (lines 86 - 95)
<https://reviews.apache.org/r/50128/#comment213380>

    I woudn't implement a `serialize()` function directly, but rather overload 
the `<<` operator. That way you can just run the global `stringify()` function 
to turn it into a string.



src/docker/docker.hpp (line 97)
<https://reviews.apache.org/r/50128/#comment213384>

    This should probably be a static function, not a member function. It should 
also return a `Try<Device>`, not a `bool`.
    
    See how things are done in for device entries in `src/linux/cgroups.*pp`



src/docker/docker.cpp (lines 389 - 392)
<https://reviews.apache.org/r/50128/#comment213392>

    Here is where you could use the new `parse()` function.



src/docker/docker.cpp (line 763)
<https://reviews.apache.org/r/50128/#comment213394>

    Here you would just use `stringify(device)` if you overload `<<` as 
suggested above.


- Kevin Klues


On Aug. 22, 2016, 10:11 a.m., Yubo Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2016, 10:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
>     https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Wrapped helper functions 'serialize()' and 'parse()' to 'Docker::Device'
> to handle data tranformation between 'Docker::Device' structure and
> string.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>