----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70735/#review215544 -----------------------------------------------------------
Fix it, then Ship it! src/common/resource_quantities.cpp Lines 558-560 (patched) <https://reviews.apache.org/r/70735/#comment302272> Why not max? ``` limit.second = std::max(limit.second - quantity.second, Value::Scalar()); ``` Do we generally try to avoid producing an negative? src/tests/resource_quantities_tests.cpp Lines 565-568 (patched) <https://reviews.apache.org/r/70735/#comment302271> Seems a little cleaner like this: ``` EXPECT_EQ(limits("cpus:0"), limits("cpus:1") - quantities("cpus:1")); EXPECT_EQ(limits("cpus:0"), limits("cpus:1") - quantities("cpus:2")); ``` etc, i.e. a `limits()` and `quantities()` labmdas, and no need for the variables and the same parsing / CHECKing that's also in the `subtract()` lambda. - Benjamin Mahler On May 28, 2019, 2:48 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70735/ > ----------------------------------------------------------- > > (Updated May 28, 2019, 2:48 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-8456 > https://issues.apache.org/jira/browse/MESOS-8456 > > > Repository: mesos > > > Description > ------- > > This patch also makes `ResourceLimits` a friend class of > `ResourceQuantities` to achieve one-pass operation complexities. > > Also added unit test. > > > Diffs > ----- > > src/common/resource_quantities.hpp 21f08c6114de4f8459c0a5f7e25d7b6bba0f060b > src/common/resource_quantities.cpp eb741e36669da4ebd1936c1d9caf7266b6105dbb > src/tests/resource_quantities_tests.cpp > ebe53be4129cceb4429c45d098a718987f557d3a > > > Diff: https://reviews.apache.org/r/70735/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
