mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 58707: Enhanced stout's Version to support prerelease and build labels.
Date Tue, 02 May 2017 20:39:10 GMT


> On April 26, 2017, 11:16 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/version.hpp
> > Lines 156-157 (patched)
> > <https://reviews.apache.org/r/58707/diff/2/?file=1699828#file1699828line160>
> >
> >     In general we try to open and close the quote on the same line if possible,
as it tends to reduce the amount of times we forget to close the quote, ditto elsewhere
> 
> Neil Conway wrote:
>     The annoying thing with moving the single quote to the next line is that you can't
easily `+` two character literals together, so you end up with
>     
>     ```
>     return Error("Identifier contains illegal character: " +
>                  string("'") + stringify(*firstInvalid) + "'");
>     ```
>     
>     which IMO is harder to read overall.

Per offline discussion, string literals can be concatenated without the + operator:

```
return Error("Identifier contains illegal character: "
             "'" + stringify(*firstInvalid) + "'");
```


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58707/#review172994
-----------------------------------------------------------


On May 2, 2017, 8 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58707/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-1987
>     https://issues.apache.org/jira/browse/MESOS-1987
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, Stout's `Version` abstraction only supported a subset of
> Semver: version numbers with three numeric components (an optional
> trailing "label" with a leading hyphen was supported but ignored).
> 
> This commit adds support for SemVer 2.0.0, which defines two additional
> optional fields: a "prerelease label" and a "build metadata label",
> e.g., "1.2.3-alpha.1+foo". Both labels consist of a series of
> dot-separated identifiers.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/version.hpp 7717c85b95d29cefe8f19f3cada4b7402d4d446f 
>   3rdparty/stout/tests/version_tests.cpp 925f3833fd8b1732c35663df3e63c161261e9bff 
> 
> 
> Diff: https://reviews.apache.org/r/58707/diff/5/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message