> On Sept. 15, 2014, 2:46 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp, lines 32-34 > > <https://reviews.apache.org/r/25597/diff/3/?file=688423#file688423line32> > > > > The program will crash if the split is non-numeric, because you'll call > > .get() on a numify operation, are you intending this to occur? > > > > A comment would be nice! > > Kapil Arya wrote: > Good catch! The question now is how to handle the situation where a > string such as "1.a.2" is passed on? Should it be considered an error or > should it be interpreted as "1.0.0"? If it should be considered an error, > how should be change the contructor to reflect that? > > Ben Mahler wrote: > You can add a 'parse' method akin to what was done in duration.hpp: > > ``` > static Try<Version> parse(const std::string& version); > ``` > > This makes the error explicit, and we no longer need the (major(), > minor(), patch()) accessor methods, since the member variables would be const.
There is one trouble with making major/minor as const fields. Glibc defines major and minor as macros and that breaks the compilation when these fields are initialized in a contructor as ": major(majorVersion), minor(minorVersion), patch(patchVersion)". The preprocessor expands them to gnu_dev_major and gnu_dev_minor respectively. The alternative solution is to go back to accessor functions and mark these fields as private. Another alternative is to rename the const fields as majorVersion, minorVersion, and patchVersion. Is there any preference here? - Kapil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53370 ----------------------------------------------------------- On Sept. 16, 2014, 1:11 a.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25597/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2014, 1:11 a.m.) > > > Review request for mesos, Adam B and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Currently there is no facility in Mesos for checking compatibility of various > Mesos components that could have been built at different times with > potentially different Mesos versions. This requirement is especially > important for doing various compatibility checks between Mesos and Mesos > modules (WIP). > > - Features major, minor, and patch numbers. > - Convenience functions for comparing two versions. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am > db9766d70adb9076946cd2b467c55636fe5f7235 > 3rdparty/libprocess/3rdparty/stout/Makefile.am > b6464de53c3873ecd0b62a08ca9aac530043ffb9 > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am > 6fa5b741bdd7f089ba93bf6fea43b9f39f8f0edb > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 5bbf829b3fa5d09a92e1d64c52c1fc7eed73fc91 > 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/25597/diff/ > > > Testing > ------- > > Added a stout test and ran make check > > > Thanks, > > Kapil Arya > >
