----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22825/#review46505 -----------------------------------------------------------
Not sure how the unit tests passed, but I have a couple of suggestions for you. 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp <https://reviews.apache.org/r/22825/#comment81908> Please comment that the string is split each time at the first character that matches any of the characters specified in delims. Otherwise, delims might be construed as a multi-character delimiter string. 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp <https://reviews.apache.org/r/22825/#comment81909> Why iterate through the rest of the string with another find_first_of() call if n==1? Check for (n==1) before calling find_first_of, and you can potentially save a lot of character comparisons. 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp <https://reviews.apache.org/r/22825/#comment81910> Shouldn't this be "baz,," not ",baz,," since you should be removing the first 3 instances of ','? 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp <https://reviews.apache.org/r/22825/#comment81911> How can tokens.size()==7 if you call splitN(6)? - Adam B On June 24, 2014, 12:27 a.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22825/ > ----------------------------------------------------------- > > (Updated June 24, 2014, 12:27 a.m.) > > > Review request for mesos, Benjamin Hindman, Dominic Hamon, and Till Toenshoff. > > > Bugs: MESOS-1522 > https://issues.apache.org/jira/browse/MESOS-1522 > > > Repository: mesos-git > > > Description > ------- > > SplitN function is stout/strings so we can choose how many tokens we want to > retreive > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 08428b8 > 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp c83156e > > Diff: https://reviews.apache.org/r/22825/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Isabel Jimenez > >
