----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26766/#review56865 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97302> IMO calling asAbsolute("") sounds more like a bug then just returning "/". 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97306> It's quite unclear what path::clean means, and have to read the tests to see what you're trying to do. When is this useful? If we're trying to clean up user configuration, I rather we expose the error instead of trying to clean up mistakes. 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97300> I don't think the result of path::join is always expected to be absolute paths? I totally see where people want to join two relative paths together. 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97309> Why does a absolute path give you two empty splits? 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97311> So an empty string means slash then? if you want to keep the beginning slash(s) why not just use slash? However, I think you have very specific use cases in mind implementing split and clean, and from the comments I can't really understand where you're coming from. Can you comment on all these methods explaining what path::clean and path::split means and does? 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/26766/#comment97310> Illegal based on? 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/26766/#comment97301> Can we rename RoundTrip and RoundTripReduce to with names that resemable more with string functions? Like VerifySplitJoin? Also just two overloads. - Timothy Chen On Oct. 15, 2014, 6:46 p.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26766/ > ----------------------------------------------------------- > > (Updated Oct. 15, 2014, 6:46 p.m.) > > > Review request for mesos and Timothy Chen. > > > Bugs: MESOS-1878 > https://issues.apache.org/jira/browse/MESOS-1878 > > > Repository: mesos-git > > > Description > ------- > > Adds 3 new functions: asAbsolute, clean, and split(). All three were > hand-coded inside of mesos files (files/files.cpp). This puts them in a > common place, and adds unit tests for their behavior. > > The functions depend on eachother somewhat, so I pulled out the declarations > to make them all forward declared. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp > 357a75a8bac497465671456aa9cd9181123cc635 > 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp > aedf93573ea89e46bf7b7b91f2258049af2fd79f > > Diff: https://reviews.apache.org/r/26766/diff/ > > > Testing > ------- > > make distcheck > > > Thanks, > > Cody Maloney > >
