----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27605/#review60651 -----------------------------------------------------------
Thanks Cody! Sorry, I was waiting for the patch to be updated, didn't see your comment about not being sure how to proceed! To make reviewbot happy here, split the patch across stout and libprocess, and *make sure to link them as dependent reviews*. Reviewbot will only test the leaves of the dependency graph. 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102026> This test could use some whitespace to differentiate the interesting patterns you're testing. As it stands it is a bit dense to read. 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102023> What's the distinction between these three tests? Seems more like a test of implicit string conversion...? 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102024> Is there a configure check for initializer lists? 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102025> Does this make sense? If not, let's call that out here with a TODO! Interestingly, we differ from python's os.path.join here. 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102027> Interestingly: >>> os.path.join("/a", "/b", "/c") '/c' 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102029> Interestingly: >>> os.path.join("/a/", "/b/", "/c/") '/c/' 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/27605/#comment102030> >>> os.path.join("a/", "/b/", "/c/") '/c/' - Ben Mahler On Nov. 5, 2014, 2:53 a.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27605/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2014, 2:53 a.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1733 and mesos-1877 > https://issues.apache.org/jira/browse/MESOS-1733 > https://issues.apache.org/jira/browse/mesos-1877 > > > Repository: mesos-git > > > Description > ------- > > Add a unit test to stout path to increase confidence when changing the code. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am > 0dc2871af7d8c6646080263c71f41db855d50be4 > 3rdparty/libprocess/3rdparty/stout/Makefile.am > 7aac3fdb864e838cdd4e0d817ed4dff14923c69d > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp > bc6920a24d920c809e36a2a3da1810e52d0db101 > 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/27605/diff/ > > > Testing > ------- > > make distcheck on Ubuntu 14.04 > > NOTE: ReviewBot is going to be unhappy about this "crossing" stout and > libprocess... I'm not sure what the correct fix for reviewbot is in this > case, since the Makefile does really cross the project bounds... > > > Thanks, > > Cody Maloney > >
