----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57824/#review169918 -----------------------------------------------------------
In terms of review formatting, it look good. You can make the review even better by filling in the `Description` field (with a beefier message summarizing the changes and reason behind the changes). And in the `Testing Done` section, you can list the steps you took to validate the patch. Usually a `make check` (or Windows equivalent) can suffice. --- I'd like to work with you on a few adjustments to this patch: * This patch basically copies chunks of the `src/environment.*`. We should therefore refactor tests up the chain. `stout::tests::internal::Environment` should be used in both `stout-tests` and `libprocess-tests` (which may have it's own set of tests that need Administrator rights, in future). * `mesos::tests::internal::Environment` should be a sub-class of `stout::tests::internal::Environment`. ^ Note: Because stout/libprocess/mesos are separate components, these changes need to be done in separate patches. 3rdparty/stout/tests/environment.cpp Lines 102-138 (patched) <https://reviews.apache.org/r/57824/#comment242622> This check doesn't need to mirror one of the tests, does it? And can you move it into the body of the `SymlinkFilter` class? --- Also, check your tab settings. We use 2 spaces per tab. 3rdparty/stout/tests/environment.cpp Lines 145-164 (patched) <https://reviews.apache.org/r/57824/#comment242632> There's no need to add Windows sections here. Tests on Posix *should* pass the filter with no problems. - Joseph Wu On March 22, 2017, 11:08 a.m., John Kordich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57824/ > ----------------------------------------------------------- > > (Updated March 22, 2017, 11:08 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. > > > Bugs: MESOS-6731 > https://issues.apache.org/jira/browse/MESOS-6731 > > > Repository: mesos > > > Description > ------- > > Filtered stout tests with symlinks when unable to create symlinks. > > > Diffs > ----- > > 3rdparty/stout/Makefile.am ebf1069eb1b787f063a2066a4db0b3f5de4a56da > 3rdparty/stout/tests/CMakeLists.txt > 4bbe713f259e7858d423dcb33956d41e62a915eb > 3rdparty/stout/tests/environment.hpp PRE-CREATION > 3rdparty/stout/tests/environment.cpp PRE-CREATION > 3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da > 3rdparty/stout/tests/os/filesystem_tests.cpp > e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc > 3rdparty/stout/tests/os/rmdir_tests.cpp > ed43b44663cbf04d7ddb449fd9f42b8de210bc6e > 3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e > > > Diff: https://reviews.apache.org/r/57824/diff/1/ > > > Testing > ------- > > > Thanks, > > John Kordich > >
