mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marco Massenzio" <ma...@mesosphere.io>
Subject Re: Review Request 36036: Add `version` string to MasterInfo
Date Wed, 01 Jul 2015 19:54:51 GMT


> On July 1, 2015, 7:48 a.m., Adam B wrote:
> > src/common/protobuf_utils.cpp, lines 147-149
> > <https://reviews.apache.org/r/36036/diff/2/?file=996269#file996269line147>
> >
> >     Could join these paragraphs. I think they're related enough, and the first one
is short.

Unless you feel strongly, I'd rather like to leave them as separate paragraphs: I find big
blocks of text less readable in apidocs.


> On July 1, 2015, 7:48 a.m., Adam B wrote:
> > src/common/protobuf_utils.cpp, lines 155-156
> > <https://reviews.apache.org/r/36036/diff/2/?file=996269#file996269line155>
> >
> >     Please capitalize the sentences/statements in your param/returns.
> >     s/the process/The process/
> >     s/a fully-formed/A fully formed/ (also, no hyphen necessary)

just FYI the javadoc style guide actually does *not* require capitalization
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide

Not sure whether we dictate otherwise in our guide.
I've capitalized here, as it was really not worth to make a fuss about :)


> On July 1, 2015, 7:48 a.m., Adam B wrote:
> > src/master/master.cpp, lines 368-369
> > <https://reviews.apache.org/r/36036/diff/2/?file=996271#file996271line368>
> >
> >     No need to log the version number here. `src/master/main.cpp` already logs the
version, build, and git tag/sha closer to the top of the master log, in a more greppable format:
> >     main.cpp:181] Build: 2015-05-05 06:16:35 by root
> >     main.cpp:183] Version: 0.22.1
> >     main.cpp:186] Git tag: 0.22.1
> >     main.cpp:190] Git SHA: d6309f92a7f9af3ab61a878403e3d9c284ea87e0
> >     
> >     And if you do include it, I don't think it needs to be inside hostname's parentheses.

removed


- Marco


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


On June 30, 2015, 11:23 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36036/
> -----------------------------------------------------------
> 
> (Updated June 30, 2015, 11:23 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2957
>     https://issues.apache.org/jira/browse/MESOS-2957
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-2957
> 
> Adds an optional version string to the `MasterInfo` protobuf,
> to simplify handling versioning in HTTP API calls.
> 
> The version string is taken from <mesos/version.hpp> which is the
> same used when starting up Master (master/main.cpp).
> 
> I've added a simple test on the back of the `MasterDetector` test,
> as this was the place where we would expect the `MasterInfo` to
> have been fully populated by real production code (as opposed to
> other places, where it is actually handled by mocks).
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 5ab3c4a305562af16af804e7ab77e576b96e468d 
>   src/common/protobuf_utils.cpp f0642ba12b914889a876d1d1c49a2c242006433c 
>   src/common/type_utils.cpp f7b21c6abc392fec0514224ff02a00e772389f5d 
>   src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
>   src/tests/master_tests.cpp 962455cc368c6e5405599d6565660d4c3fd0fc22 
> 
> Diff: https://reviews.apache.org/r/36036/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


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