----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27175/#review58954 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100180> These TODO comments should be updted since this adds a 4th component if nothing else 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100184> labelSplit? 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100185> version? versionChunks? versionComponents? the name split isn't very helpful. 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100188> If we go with semantic versioning: Rule 10: "Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows: identifiers consisting of only digits are compared numerically and identifiers with letters or hyphens are compared lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers." which this doesn't follow. If we want to pull out the structure of the naming we know / do (rc) then definitely we can/should. According to SemVer we should call this pre-release in general and we would then have a helper in Version which tells you whether or not there is a pre-release. It is likely that I'll use the pre-release versions for tracking mesos testing cluster builds for continuous integration (I can forsee the version string 0.20.1-2014.10.28.3+debug) 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100189> Why not strings::lower(secondarySplit[0]) so that these cases condense? 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100190> Shouldn't tag = UNKNOWN here? 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100191> shouldn't the versionString contain the tag if one is specified? Why isn't not passing in the original version string a programmer error / assertion failure? 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100192> We really need some form of "more or less equal" / some level of the fields match (major + minor + patch). Most users shouldn't care about tags for checking to enable / disable features. If they are getting that specific day to day development will likely break them. I don't think this should compare tags, but I see the argument either way. 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp <https://reviews.apache.org/r/27175/#comment100194> The tag comparison here is incorrect. If a tag is given at all, that should always be lower level than tag == RELEASE. I'd much rather see that logic laid out here than being implicit in the ordering of the members of the enum. Same goes for operator > 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp <https://reviews.apache.org/r/27175/#comment100195> Can you add a test for release > rc/unknown tag? - Cody Maloney On Oct. 24, 2014, 10:55 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27175/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2014, 10:55 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1987 > https://issues.apache.org/jira/browse/MESOS-1987 > > > Repository: mesos-git > > > Description > ------- > > Now it can parse strings like "1.2.3-rc4". Other tags are still > discarded. > > Updated os::release() to use Version::parse() instead of sscanf. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 5d3cbba289d8f9d57c2fd49dd6f72225bb2fb8c2 > 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp > 090fcf09dd96538a8748cf4443d150911e2c0d27 > 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp > e8f8358f3c113b4e21e30cb5e9d6a3d229191484 > > Diff: https://reviews.apache.org/r/27175/diff/ > > > Testing > ------- > > Enhanced version tests and ran make check. > > > Thanks, > > Kapil Arya > >
