You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2017/05/03 18:28:13 UTC
Review Request 58971: Switched to using unsigned types to represent
versions in stout.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58971/
-----------------------------------------------------------
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
Re: Review Request 58971: Switched to using unsigned types to
represent versions in stout.
Posted by Neil Conway <ne...@gmail.com>.
> On May 3, 2017, 7:41 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/version.hpp
> > Lines 282-283 (original), 283-284 (patched)
> > <https://reviews.apache.org/r/58971/diff/1/?file=1707195#file1707195line283>
> >
> > 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.
Fair enough -- I decided to go with `uint32_t`.
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58971/#review173797
-----------------------------------------------------------
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
>
>
Re: Review Request 58971: Switched to using unsigned types to
represent versions in stout.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 58971: Switched to using unsigned types to
represent versions in stout.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58971/
-----------------------------------------------------------
(Updated May 3, 2017, 7:55 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Tweak comment.
Repository: mesos
Description
-------
Switched to using unsigned types to represent versions in stout.
Diffs (updated)
-----
3rdparty/stout/include/stout/version.hpp 7717c85b95d29cefe8f19f3cada4b7402d4d446f
Diff: https://reviews.apache.org/r/58971/diff/3/
Changes: https://reviews.apache.org/r/58971/diff/2-3/
Testing
-------
`make check`
Thanks,
Neil Conway
Re: Review Request 58971: Switched to using unsigned types to
represent versions in stout.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58971/
-----------------------------------------------------------
(Updated May 3, 2017, 7:51 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Switch to using `uint32_t`.
Repository: mesos
Description
-------
Switched to using unsigned types to represent versions in stout.
Diffs (updated)
-----
3rdparty/stout/include/stout/version.hpp 7717c85b95d29cefe8f19f3cada4b7402d4d446f
Diff: https://reviews.apache.org/r/58971/diff/2/
Changes: https://reviews.apache.org/r/58971/diff/1-2/
Testing
-------
`make check`
Thanks,
Neil Conway