----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66924/#review202436 -----------------------------------------------------------
src/master/master.hpp Line 442 (original), 442 (patched) <https://reviews.apache.org/r/66924/#comment284252> The original pattern here seems to have been to always consitently break after the opening paren. Let's not change that. src/master/master.hpp Line 506 (original), 505 (patched) <https://reviews.apache.org/r/66924/#comment284254> See above. src/master/master.hpp Line 513 (original), 511 (patched) <https://reviews.apache.org/r/66924/#comment284255> Break after `(`. src/tests/storage_local_resource_provider_tests.cpp Lines 3500 (patched) <https://reviews.apache.org/r/66924/#comment284261> The behavior described in the comment above seems rather simple, but we use a pretty complicated setup below. This not only leads to us writing too much code unrelated to the thing under test, but also introduces unneeded dependencies on behavior we don't care about (here) -- e.g., I don't think this test depends on a SLRP. I would also much prefer a non-`ROOT` test so we run this as often as possible (I suspect developers do not regularly run `make check` as root on their development machines). I would suggest to reimplement the test with a `MockResourceProvider` if we do really need a `CREATE_VOLUME` operation below -- but it seems we should be able to perform this test with even non-RP operations which would simplify things further. See e.g., the test added in r/66908 which needs 60% less test code. - Benjamin Bannier On May 4, 2018, 2 a.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66924/ > ----------------------------------------------------------- > > (Updated May 4, 2018, 2 a.m.) > > > Review request for mesos, Benjamin Bannier and Greg Mann. > > > Bugs: MESOS-8784 > https://issues.apache.org/jira/browse/MESOS-8784 > > > Repository: mesos > > > Description > ------- > > Made the master include the operation ID in OPERATION_DROPPED updates. > > > Diffs > ----- > > src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 > src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 > src/tests/storage_local_resource_provider_tests.cpp > ccb114aacc904998dfeef4128f6d38dfb3c865c7 > > > Diff: https://reviews.apache.org/r/66924/diff/1/ > > > Testing > ------- > > `make check` on GNU/Linux. > > > The test in this patch fails without the corresponding master change, but > succeeds with it applied. > > > Thanks, > > Gaston Kleiman > >
