----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55910/#review163567 -----------------------------------------------------------
Fix it, then Ship it! src/master/allocator/mesos/hierarchical.cpp (lines 381 - 385) <https://reviews.apache.org/r/55910/#comment235018> Not yours, but do you know what's going on here? The comment doesn't seem to match the code. src/master/allocator/mesos/hierarchical.cpp (line 632) <https://reviews.apache.org/r/55910/#comment235021> Seems like we could've just kept: ```cpp const string& role = frameworks.at(frameworkId).role; ``` and used `role` rather than `framework.role` below. src/master/allocator/mesos/hierarchical.cpp (line 1424) <https://reviews.apache.org/r/55910/#comment235022> Did you mean `const Owned<Sorter>&` here? I'd prefer to not introduce more places where `Owned` is being used like a `Shared`. or maybe even: ```cpp Sorter& frameworkSorter = *frameworkSorters.at(role); ``` - Michael Park On Jan. 24, 2017, 6:40 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55910/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2017, 6:40 p.m.) > > > Review request for mesos and Michael Park. > > > Repository: mesos > > > Description > ------- > > Currently, a lof of code in the allocator makes use of the `[]` > operator to access the agents, frameworks and sorters, which > can lead to subtle bugs where insertion was unintended. > > With this change, a few const functions can be marked as such. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 9b66c23f26b37c02ed850c06c4518ea99078b02d > src/master/allocator/mesos/hierarchical.cpp > c2211be7458755aeb91ef078e4bfe92ac474044a > > Diff: https://reviews.apache.org/r/55910/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Mahler > >
