----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59194/#review174772 -----------------------------------------------------------
src/common/resources.cpp Lines 1414-1416 (patched) <https://reviews.apache.org/r/59194/#comment248008> Line 1 and line 3 have some redundancy: "ensure operation removes volume vs. ensure the volume is removed". I feel that the error message below could use a few more words to be clearer and if we do that probably the 2nd sentence in this comment could probably be removed. src/common/resources.cpp Lines 1417 (patched) <https://reviews.apache.org/r/59194/#comment248038> If this check fails we would return an Error directly so doesn't look like we need this temp var? src/common/resources.cpp Lines 1422 (patched) <https://reviews.apache.org/r/59194/#comment248048> ``` stringify(volume) + " cannot be removed due to additional shared copies" ``` ? src/tests/resources_tests.cpp Lines 2573-2575 (patched) <https://reviews.apache.org/r/59194/#comment248010> Why are we testing CREATE? This test can just literally be a DESTROY operation on a resources object with two volumes (which is simpler) right? - Jiang Yan Xu On May 11, 2017, 1:21 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59194/ > ----------------------------------------------------------- > > (Updated May 11, 2017, 1:21 p.m.) > > > Review request for mesos, James Peach and Jiang Yan Xu. > > > Bugs: MESOS-7403 > https://issues.apache.org/jira/browse/MESOS-7403 > > > Repository: mesos > > > Description > ------- > > Ensure that applying `DESTROY` operation removes this volume. This is > needed to handle `DESTROY` of shared volumes to ensure that the > resulted `Resources` object has this `Resource` removed. > > > Diffs > ----- > > src/common/resources.cpp f6f02eb0a7ca05ab8fd9a670f32428862009b7d5 > src/tests/resources_tests.cpp 4cf320d802a749f1419ac5b9f63b6c73b0c974be > src/v1/resources.cpp cad069defb34d5ccc20a0e083eb88cb80f70e415 > > > Diff: https://reviews.apache.org/r/59194/diff/1/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Anindya Sinha > >
