> On Dec. 17, 2014, 12:53 a.m., Ben Mahler wrote: > > include/mesos/resources.hpp, line 241 > > <https://reviews.apache.org/r/29128/diff/1/?file=793377#file793377line241> > > > > It might not be clear what "no transformation will be applied" means, > > error? > > > > How about: > > s/no transformation will be applied/no change occurs/
Invalid after the discussion. > On Dec. 17, 2014, 12:53 a.m., Ben Mahler wrote: > > src/common/resources.cpp, lines 918-922 > > <https://reviews.apache.org/r/29128/diff/1/?file=793378#file793378line918> > > > > How about: > > > > ``` > > if (resources.contains(disk)) { > > return resources; // No-op: already acquired! > > } Invalid after the discussion. > On Dec. 17, 2014, 12:53 a.m., Ben Mahler wrote: > > src/common/resources.cpp, line 920 > > <https://reviews.apache.org/r/29128/diff/1/?file=793378#file793378line920> > > > > Is Resources() here necessary? It looks like there is an operator for > > just doing: > > > > ``` > > if (resources.contains(disk)) { > > ... > > } > > ``` > > > > Via this: > > ``` > > bool Resources::contains(const Resource& that) const; > > ``` We dont' have 'contains' for Resource object. I'll add one in a following patch. Drop a TODO here. > On Dec. 17, 2014, 12:53 a.m., Ben Mahler wrote: > > src/common/resources.cpp, line 932 > > <https://reviews.apache.org/r/29128/diff/1/?file=793378#file793378line932> > > > > Might want to wrap the ID in single quotes, unless we're heavily > > sanitizing it (e.g. do we allow whitespace?) No longer valid. > On Dec. 17, 2014, 12:53 a.m., Ben Mahler wrote: > > src/tests/resources_tests.cpp, line 952 > > <https://reviews.apache.org/r/29128/diff/1/?file=793379#file793379line952> > > > > Just curious, where will we catch a duplicate container path under the > > same executor? > > > > Do you have an integration test for this at least? Let's keep track of > > this extra case. Will check that in master. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29128/#review65262 ----------------------------------------------------------- On Dec. 16, 2014, 11:56 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29128/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2014, 11:56 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2030 > https://issues.apache.org/jira/browse/MESOS-2030 > > > Repository: mesos-git > > > Description > ------- > > See summary. Seprated out from https://reviews.apache.org/r/28720/ > > > Diffs > ----- > > include/mesos/resources.hpp 296553a > src/common/resources.cpp 9bf7ae9 > src/tests/resources_tests.cpp bac092e > > Diff: https://reviews.apache.org/r/29128/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
