mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 25597: Added a version checker class to stout.
Date Tue, 16 Sep 2014 19:36:23 GMT

-----------------------------------------------------------
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
> 
>


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