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 58971: Switched to using unsigned types to represent versions in stout.
Date Wed, 03 May 2017 19:41:51 GMT

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


Fix it, then Ship it!





3rdparty/stout/include/stout/version.hpp
Lines 282-283 (original), 283-284 (patched)
<https://reviews.apache.org/r/58971/#comment246831>

    Looks like we're a bit inconsistent about this in the code base, but it seems to me that
being explicit about this being an 'int' is more readable than relying on the implicit 'int'
(i.e. reader doesn't have to know that the equivalent type of 'unsigned' is 'unsigned int').
    
    That would mean we only use the 'Equivalent Type's in the table here (outside of the fixed
width types like uint32_t): http://en.cppreference.com/w/cpp/language/types
    
    Which also seems like an easy code style rule to enforce?
    
    Alternatively, uint32_t or uint64_t works here if you want the width to be clear.


- Benjamin Mahler


On May 3, 2017, 6:28 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58971/
> -----------------------------------------------------------
> 
> (Updated May 3, 2017, 6:28 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Switched to using unsigned types to represent versions in stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/version.hpp 7717c85b95d29cefe8f19f3cada4b7402d4d446f 
> 
> 
> Diff: https://reviews.apache.org/r/58971/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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