----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48314/#review137643 -----------------------------------------------------------
Given that we already have `os::ls()` I don't see this as strictly necessary but I guess we can benefit from the early return of the method compared to `os::ls`. 3rdparty/stout/include/Makefile.am (line 69) <https://reviews.apache.org/r/48314/#comment203024> empty goes before exists. 3rdparty/stout/include/stout/os/empty.hpp (lines 24 - 25) <https://reviews.apache.org/r/48314/#comment203032> This and other places in this file that use two blank lines should all use a single blank line. Double blank lines separate outer elements but this file defines one method. We don't use two blank lines before the first outer element or after the last. I do see inconsistencies in ls.hpp though... 3rdparty/stout/include/stout/os/empty.hpp (lines 27 - 28) <https://reviews.apache.org/r/48314/#comment203033> One blank line. 3rdparty/stout/include/stout/os/empty.hpp (line 33) <https://reviews.apache.org/r/48314/#comment202836> The codebase rarely tests nullptr this way, switch to `root == nullptr`? 3rdparty/stout/include/stout/os/empty.hpp (line 35) <https://reviews.apache.org/r/48314/#comment202835> Use ErrnoError. We don't know what error it is right? 3rdparty/stout/include/stout/os/empty.hpp (line 39) <https://reviews.apache.org/r/48314/#comment202832> s/NULL/nullptr/ 3rdparty/stout/include/stout/os/empty.hpp (lines 50 - 51) <https://reviews.apache.org/r/48314/#comment203023> We could also reach here due to an error right? Check `error`? I think it also works on Windows? 3rdparty/stout/include/stout/os/empty.hpp (lines 55 - 56) <https://reviews.apache.org/r/48314/#comment203034> One blank line. 3rdparty/stout/include/stout/os/empty.hpp (lines 58 - 59) <https://reviews.apache.org/r/48314/#comment203035> One blank line. - Jiang Yan Xu On June 13, 2016, 2:03 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48314/ > ----------------------------------------------------------- > > (Updated June 13, 2016, 2:03 p.m.) > > > Review request for mesos, Neil Conway 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 > >
