> On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > include/mesos/resources.hpp, lines 190-198 > > <https://reviews.apache.org/r/29018/diff/2/?file=793220#file793220line190> > > > > Maybe this could be a bit more succinct: > > > > ``` > > // This is an abstraction for describing a transformation that can > > // be applied to Resources. Transformations cannot not alter the > > // quantity, or the static role of the resources. > > // > > // TODO(jieyu): Enforce the transformation invariants fully! > > class Transformation > > { > > public: > > ... > > > > // Returns the result of the transformation, applied to 'resources'. > > // Returns an Error if the transformation cannot be applied. > > Try<Resources> operator () (const Resources& resources) const; > > }; > > ```
Done. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > include/mesos/resources.hpp, lines 210-212 > > <https://reviews.apache.org/r/29018/diff/2/?file=793220#file793220line210> > > > > Maybe mention the all-or-nothing nature: > > > > ``` > > // Represents a sequence of transformations, the transformations > > // are applied in an all-or-nothing manner. > > class CompositeTransformation : public Transformation > > { > > }; > > ``` Done. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > include/mesos/resources.hpp, line 236 > > <https://reviews.apache.org/r/29018/diff/2/?file=793220#file793220line236> > > > > You need an include for vector now? DOne. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > src/common/resources.cpp, line 33 > > <https://reviews.apache.org/r/29018/diff/2/?file=793221#file793221line33> > > > > Unused? Removed. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > src/common/resources.cpp, line 860 > > <https://reviews.apache.org/r/29018/diff/2/?file=793221#file793221line860> > > > > "transformations"? Fixed. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > src/common/resources.cpp, lines 864-885 > > <https://reviews.apache.org/r/29018/diff/2/?file=793221#file793221line864> > > > > Thanks, could this be: > > > > ``` > > Try<Resources> applied = apply(resources); > > > > if (applied.isSome()) { > > // Additional checks. > > } > > > > return applied; > > ``` Done. > On Dec. 16, 2014, 9:42 p.m., Ben Mahler wrote: > > src/common/resources.cpp, line 895 > > <https://reviews.apache.org/r/29018/diff/2/?file=793221#file793221line895> > > > > newline here? Done. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29018/#review65241 ----------------------------------------------------------- On Dec. 16, 2014, 9:10 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29018/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2014, 9:10 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2030 > https://issues.apache.org/jira/browse/MESOS-2030 > > > Repository: mesos-git > > > Description > ------- > > Added abstraction for resources transformation. Split from > https://reviews.apache.org/r/28720/. > > > Diffs > ----- > > include/mesos/resources.hpp 296553af93ec8457dad04ac018f03f36df654fdc > src/common/resources.cpp 9bf7ae9148d6db2829cc2866ac048fe018ae2c92 > > Diff: https://reviews.apache.org/r/29018/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
