----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49352/#review140220 -----------------------------------------------------------
Fix it, then Ship it! src/common/parse.hpp (lines 143 - 144) <https://reviews.apache.org/r/49352/#comment205536> How about a comment similar to Kevin's TODO on the vector parse above? ``` // NOTE: Strings in the set cannot contain commas, since that // is the delimiter and we provide no way to escape it. // // TODO(klueska): Generalize this parser to take any comma separated // list and convert it to its appropriate type (i.e., not just for // unsigned ints). ``` src/common/parse.hpp (line 149) <https://reviews.apache.org/r/49352/#comment205534> We try to avoid using integers as booleans in favor of explicitly checking the condition you care about, so: ``` if (result.count(token) > 0) { ``` src/common/parse.hpp (line 150) <https://reviews.apache.org/r/49352/#comment205535> How about: ``` return Error("Duplicate token '" + token + "'"); ``` - Benjamin Mahler On June 30, 2016, 7:43 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49352/ > ----------------------------------------------------------- > > (Updated June 30, 2016, 7:43 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-5743 > https://issues.apache.org/jira/browse/MESOS-5743 > > > Repository: mesos > > > Description > ------- > > Added a flag parser for std::set<std::string>. > > > Diffs > ----- > > src/common/parse.hpp 19e56dbeb765f8bec92e0a3615f6f7c12466fa9e > > Diff: https://reviews.apache.org/r/49352/diff/ > > > Testing > ------- > > make > make check > > > Thanks, > > Guangya Liu > >
