----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51999/#review149624 -----------------------------------------------------------
include/mesos/resources.hpp (lines 60 - 64) <https://reviews.apache.org/r/51999/#comment217350> We can get rid of this forward declaration if we can get rid of the internal convertJSON. include/mesos/resources.hpp (line 183) <https://reviews.apache.org/r/51999/#comment217356> Use `/**`. include/mesos/resources.hpp (line 201) <https://reviews.apache.org/r/51999/#comment217287> This is too similar to ``` static Try<Resources> parse( const std::string& text, const std::string& defaultRole = "*"); ``` I was originally suggesting moving over convertJSON but I guess it's more suitable here in the following form: ``` Try<vector<Resource>> fromJSONString( const std::string& text, const std::string& defaultRole) ``` Similarly for simple flat strings: ``` Try<vector<Resource>> fromSimpleString( const std::string& text, const std::string& defaultRole) ``` When we have both, then ``` static Try<Resources> parse( const std::string& text, const std::string& defaultRole = "*"); ``` becomes a mere convenience method that calls the above two methods in sequence. The convenience method can be simply documented as: ``` /** * Returns Resources from an input string. * * Parses Resources from text in the form of a JSON array (see * `fromJSONString()`). If that fails, parses text in the simple * string form (see `fromSimpleString()`). */ ``` How does it sound? - Jiang Yan Xu On Sept. 19, 2016, 3:42 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51999/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 3:42 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-6062 > https://issues.apache.org/jira/browse/MESOS-6062 > > > Repository: mesos > > > Description > ------- > > During parsing of resources in the startup flags of mesos agent, all > the valid resources (including empty resources) are accumulated in a > vector of Resource protobufs. > > > Diffs > ----- > > include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 > include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 > src/common/resources.cpp 4602bff22ec7ff77f069056085ad92bfa04595f3 > src/slave/containerizer/containerizer.cpp > d46882baa904fd439bffb23c324828b777228f1c > src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a > src/v1/resources.cpp 7c4db44d4c31a8295cc16b0ce1ef5adf314e6ac6 > > Diff: https://reviews.apache.org/r/51999/diff/ > > > Testing > ------- > > > Thanks, > > Anindya Sinha > >
