----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48314/#review136631 -----------------------------------------------------------
3rdparty/stout/include/stout/os/empty.hpp (line 34) <https://reviews.apache.org/r/48314/#comment201725> Treating a non-existent directory as "empty" doesn't strike me as good interface design: these are two very different situations, and many callers would want to distinguish between them. What about returning `Try<bool>` instead? 3rdparty/stout/include/stout/os/empty.hpp (line 38) <https://reviews.apache.org/r/48314/#comment201722> Initializing this to `NULL` explicitly is misleading: the variable is always assigned to before it is read, so I'd prefer to leave it uninitialized. Also, we now use `nullptr` instead of `NULL`. 3rdparty/stout/tests/os/rmdir_tests.cpp (line 62) <https://reviews.apache.org/r/48314/#comment201724> Seems redundant to check both `listfiles` and `os::empty`, here and elsewhere. 3rdparty/stout/tests/os/rmdir_tests.cpp (line 138) <https://reviews.apache.org/r/48314/#comment201727> If we're going to check `tmpdir` here, we should check `newDirectoryAbsolutePath` as well. - Neil Conway On June 7, 2016, 5:53 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48314/ > ----------------------------------------------------------- > > (Updated June 7, 2016, 5:53 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-5448 > https://issues.apache.org/jira/browse/MESOS-5448 > > > Repository: mesos > > > Description > ------- > > Added os::empty(path) to check if contents in a directory is empty. > > > Diffs > ----- > > 3rdparty/stout/include/Makefile.am 7f31582c176e653873402bd3f19b0c0195503d45 > 3rdparty/stout/include/stout/os.hpp > 53b00932693fba7cf6514da6a519269a904de345 > 3rdparty/stout/include/stout/os/empty.hpp PRE-CREATION > 3rdparty/stout/tests/os/rmdir_tests.cpp > a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f > > Diff: https://reviews.apache.org/r/48314/diff/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Anindya Sinha > >
