> On Nov. 16, 2014, 1:10 a.m., Michael Park wrote: > > src/common/resources.cpp, line 391 > > <https://reviews.apache.org/r/28091/diff/2/?file=765012#file765012line391> > > > > +1 > > Ben Mahler wrote: > Are you +1'ing my comments? If so, to keep threads intact you have to > comment on the main review page. > > It looks like some of your other comments were meant to be replies to > mine?
Yeah... just noticed whatever I did to try to +1 on your comments didn't work. Will know for next time. > On Nov. 16, 2014, 1:10 a.m., Michael Park wrote: > > src/common/resources.cpp, line 738 > > <https://reviews.apache.org/r/28091/diff/2/?file=765012#file765012line738> > > > > We should keep the `size` check as long as it's still valid since it's > > a trivial check that allows us to exit early. > > Jie Yu wrote: > I prefer not because what does 'size()' mean to Resources object? It's > not clear. So I would rather not to introduce this interface. Ok. > On Nov. 16, 2014, 1:10 a.m., Michael Park wrote: > > src/common/resources.cpp, line 387 > > <https://reviews.apache.org/r/28091/diff/2/?file=765012#file765012line387> > > > > In this case, it doesn't __need__ to be. But `Resources` has a member > > variable named `resources` which would be shadowed if this local variable > > had the same name. We should stay away from shadowing names as it's very > > error-prone and can be difficult to debug. > > Jie Yu wrote: > Fixed This was also supposed to be reply to BenM's comment. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28091/#review61673 ----------------------------------------------------------- On Nov. 17, 2014, 8:35 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28091/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2014, 8:35 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-1974 > https://issues.apache.org/jira/browse/MESOS-1974 > > > Repository: mesos-git > > > Description > ------- > > See summary. Always combine Resource objects in Resources and disallow > invalid/zero Resource objects. > > > Diffs > ----- > > include/mesos/resources.hpp 0e37170262d3470570a3436b7835bb1d4a121056 > src/common/resources.cpp e9a0c85dc3748aa635843d98cd5993d5648ec5c2 > src/examples/low_level_scheduler_libprocess.cpp > 89b43181c9ea012e04018482fb9edd2f8091d63e > src/examples/low_level_scheduler_pthread.cpp > e5cd48a76e201174af1e3a5f45576a342738dceb > src/examples/test_framework.cpp e87198bba7c60005d01e6fd58965ac322a3a53e3 > src/master/drf_sorter.cpp 54649003745721e75e715b9d2e950e1b38f6b9db > src/master/hierarchical_allocator_process.hpp > 31dfb2cc86e2e74da43f05fbada10976ce65f3e4 > src/master/http.cpp 31899334b905f83a2305c51c4bfefbee623e697e > src/master/master.cpp 83c2f8a94c00a1b07c5e6ab4e10404e0b3fdf2da > src/slave/containerizer/containerizer.cpp > f234835180b42331f731d92cd2ad7bd56b6efa70 > src/tests/gc_tests.cpp 8618ae12b337bea19a82dec52455cd19cc735d89 > src/tests/mesos.hpp c1d64a73ff2312a10d4e809072d219e60c28a76f > src/tests/resource_offers_tests.cpp > 43820b0a709b6e8643940b70183e277000d4ba35 > src/tests/resources_tests.cpp 3e50889ff2433930bd137a9d836d0a8bfc2d0388 > > Diff: https://reviews.apache.org/r/28091/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
