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

Ship it!



src/tests/environment.cpp (line 365)
<https://reviews.apache.org/r/42374/#comment175964>

    We should really add our own `os::which` so that we can easily 
differentiate the error cases here. In particular, what if just `os::system` 
fails? or `which` fails but `logrotate` is still accesible on the path? Having 
something like a `os::which` which returns a `Result`, either some (it exists 
on the path), none (it doesn't exist on the path), or `Error` (there was an 
error determining that information) would be great. While an `Error` or none 
would still do the same thing here, it would probably give a better error 
message.
    
    Feel free to add these kinds of primitives on the journey in the future!


- Benjamin Hindman


On Jan. 16, 2016, 12:54 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42374/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4136
>     https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `logrotate` binary is used by the non-default `RotatingContainerLogger` 
> module.  On some systems, `logrotate` is not provided by default.
> 
> 
> Diffs
> -----
> 
>   src/tests/environment.cpp 20218a086baefcefb310eb45ed9024e5425ce787 
> 
> Diff: https://reviews.apache.org/r/42374/diff/
> 
> 
> Testing
> -------
> 
> See later reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to