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


Thanks jie, this was my fault. I did the big Try<Nothing> refactor and didn't 
realize that you didn't want to call the os:: recursive functions.

Can you provide background on why this breaks the tests?


third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/7338/#comment25643>

    I find it surprising that we had mkdir behaving as mkdir -p. I think mkdirp 
makes more sense, but changing that is yet another big refactor, so maybe add a 
TODO.
    
    For now, I think you could just leave os::mkdir as is, and do ::mkdir in 
your cgroups code as before, since the note you added clears things up.



third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/7338/#comment25644>

    I like your note above about "NOT deleting recursively", and I think this 
case is special enough for you to use ::rmdir inline in the cgroups code rather 
adding the flag here.
    
    I missed it in the refactor since I wasn't sure why you used ::rmdir 
instead of os::rmdir, but the notes you added will make sure no one else makes 
this mistake :)


- Ben Mahler


On Sept. 28, 2012, 5:33 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7338/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2012, 5:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> The recent refactor changes break the assumptions in the cgroups code.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp cdafe6e 
>   third_party/libprocess/include/stout/os.hpp 13dbc71 
> 
> Diff: https://reviews.apache.org/r/7338/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> Tested on my vm (latest ubuntu 12.04)
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to