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


Almost there!!

I think we can get away with not needing to expose major, minor, and patch for 
now. If you look at the use-case for os::Release, we only needed the comparison 
operator.
That would avoid the majorVersion clunkiness, what do you think?


3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp
<https://reviews.apache.org/r/25597/#comment93271>

    We've tried to stick with // style comments, can you change this one?



3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp
<https://reviews.apache.org/r/25597/#comment93276>

    Can you move this down to where it is used?



3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp
<https://reviews.apache.org/r/25597/#comment93275>

    s/. M/; m/
    
    I don't think you need the <int>'s here on the stringify calls.
    
    Also, we don't use periods at the end of any of our log lines. There are 
two reasons for this, (1) the code needed is clumsy in some situations, and (2) 
it's tricky with nested errors. For example, who's responsibility is it here to 
end with a period? If the caller does the following we end up with two periods!
    
    ```
    LOG(ERROR) << "Failed to parse: " << error.get() << ".";
    ```



3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp
<https://reviews.apache.org/r/25597/#comment93269>

    Two newlines between top level definitions.



3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp
<https://reviews.apache.org/r/25597/#comment93270>

    Ditto here.


- Ben Mahler


On Sept. 16, 2014, 6:48 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25597/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 6:48 p.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
> 
>

Reply via email to