----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70169/#review213801 -----------------------------------------------------------
Reviewed simultaneously with https://reviews.apache.org/r/70168/, suggest to merge the trivial code movements in a single patch. Please see comment there as well. src/resource_provider/storage/provider.cpp Line 3734 (original), 3140 (patched) <https://reviews.apache.org/r/70169/#comment299829> Preexisting, but isn't this already covered by above `operations_dropped` counter for `UNKNOWN`? src/resource_provider/storage/provider_process.hpp Line 122 (original), 119 (patched) <https://reviews.apache.org/r/70169/#comment299827> Preexisting, but I don't understand the value this comment adds to this decl. If anything, we could make the parameter `const` when actually defining this function, but it seems leaky to expose this in the declaration. src/resource_provider/storage/provider_process.hpp Line 369 (original), 344 (patched) <https://reviews.apache.org/r/70169/#comment299826> Preexisting condition, but should we make this class non-copyable? src/resource_provider/storage/provider_process.hpp Line 402 (original), 373 (patched) <https://reviews.apache.org/r/70169/#comment299828> Composition instead of inheritance seems to work just fine here, let's do that instead. - Benjamin Bannier On March 12, 2019, 9:43 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70169/ > ----------------------------------------------------------- > > (Updated March 12, 2019, 9:43 p.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9632 > https://issues.apache.org/jira/browse/MESOS-9632 > > > Repository: mesos > > > Description > ------- > > Refactored SLRP to use `ServiceManager`. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > fea623c292158deb1b4b4b9ab1ac208031471519 > src/resource_provider/storage/provider_process.hpp > a5536b3d735e01eb1c4dc52d0602d973155f3c93 > > > Diff: https://reviews.apache.org/r/70169/diff/2/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
