> On April 13, 2018, 12:27 a.m., Greg Mann wrote: > > include/mesos/mesos.proto > > Lines 1975 (patched) > > <https://reviews.apache.org/r/66049/diff/6/?file=1994606#file1994606line1975> > > > > Hmm I wonder if we should just use a `double` here? Does the > > `Value.Scalar` type provide some benefit? > > Chun-Hung Hsiao wrote: > From the API perspective, `double` is easier to use, but `Value.Scalar` > is more consistent with how we represesnt a disk resource. > > From the implementation perspective, I prefer `Value.Scalar` because it's > arithmetic operators will do fixed-point conversions for me so I won't forget > that. > > Greg Mann wrote: > My recommendation would be to prioritize easy-of-use in the API over > ease-of-implementation when possible :) > > In the implementation, as long as we toss the provided double into a > `Scalar::Value` before we do anything with it, I think we could get the same > benefit? > > Zhitao Li wrote: > I feel that `Scalar` is actually more consistent than `double` here. I do > not see either API is much easier to use.
Why is `Scalar` more consistent? I think that `double` would be simpler because it eliminates the nested `value` field, which is not intuitive. Is there any reason to make this a `Scalar` other than the fact that it makes our implementation simpler? Do we have other APIs which accept a `Value.Scalar` for similar purposes? - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review201067 ----------------------------------------------------------- On April 13, 2018, 5:20 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66049/ > ----------------------------------------------------------- > > (Updated April 13, 2018, 5:20 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > Added offer operation to grow and shrink persistent volumes. > > > Diffs > ----- > > include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 > include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 > src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 > src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 > src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a > src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e > src/tests/persistent_volume_tests.cpp > 4edf781711d9efdb994114aeb6289b6af750b87a > src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d > > > Diff: https://reviews.apache.org/r/66049/diff/8/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
