----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43561/#review124545 -----------------------------------------------------------
src/common/values.cpp (line 612) <https://reviews.apache.org/r/43561/#comment187159> Not yours: We shouldn't capture temporaries by reference. Either: 1) capture by value: `const vector<string> tokens = ...` 2) iterator over the result: ``` foreach (const string& token, strings::tokenize(...)) { ``` I mention it because you copy the pattern below. src/common/values.cpp (lines 612 - 626) <https://reviews.apache.org/r/43561/#comment187160> Should we add a benchmark here to understand the effect of now having 3 levels of `tokenize`? Is this code in any critical paths? src/common/values.cpp (line 613) <https://reviews.apache.org/r/43561/#comment187161> @benm I wish we had support for iterating over these splicers eg: `foreachtoken(temp, ",\n", [](const string& token) { ... });` These helpers become rather heavy (at 3 nested) considering their utility. - Joris Van Remoortere On March 19, 2016, 10:08 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43561/ > ----------------------------------------------------------- > > (Updated March 19, 2016, 10:08 a.m.) > > > Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van > Remoortere. > > > Bugs: MESOS-4627 > https://issues.apache.org/jira/browse/MESOS-4627 > > > Repository: mesos > > > Description > ------- > > Improve Ranges parsing to handle single values. > > > Diffs > ----- > > src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 > src/tests/resources_tests.cpp a545100522bf4b1f03e50656d461b3cda6b41e11 > src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 > > Diff: https://reviews.apache.org/r/43561/diff/ > > > Testing > ------- > > make > make check GTEST_FILTER=~"*" > ./src/mesos-test > > > Thanks, > > Klaus Ma > >
