----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65074/#review195286 -----------------------------------------------------------
Fix it, then Ship it! src/slave/compatibility.cpp Line 156 (original), 135 (patched) <https://reviews.apache.org/r/65074/#comment274464> s/by Mesos//. Seems redundant? src/slave/compatibility.cpp Line 203 (original), 196 (patched) <https://reviews.apache.org/r/65074/#comment274465> ditto. src/tests/slave_compatibility_tests.cpp Lines 203 (patched) <https://reviews.apache.org/r/65074/#comment274466> Move this to #200 and say // This is a regression test for MESOS-8410. Or Moe this to #345 add another case at the end of the test to add one more disk and make sure that doesn't trip up additive policy either. src/tests/slave_compatibility_tests.cpp Lines 215-230 (patched) <https://reviews.apache.org/r/65074/#comment274467> do you really need these many disks for the regression test. are 2 not enough? - Vinod Kone On Jan. 11, 2018, 8:08 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65074/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2018, 8:08 p.m.) > > > Review request for mesos, James Peach and Vinod Kone. > > > Bugs: MESOS-8410 > https://issues.apache.org/jira/browse/MESOS-8410 > > > Repository: mesos > > > Description > ------- > > The case where multiple resources have the same name > was not handled correctly, and could result in false > negatives. > > To fix this, we now consider all resource metadata in addition to the name > when searching for a matching resource in the previous configuration. > > > Diffs > ----- > > include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 > src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 > src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 > src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 > src/tests/slave_compatibility_tests.cpp > ab5ed29f43c593dbd7d90c235aa304b581d932a2 > > > Diff: https://reviews.apache.org/r/65074/diff/1/ > > > Testing > ------- > > * Several new unit tests added in the review > * `make check` > > > Thanks, > > Benno Evers > >
