mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Clemmer" <clemmer.alexan...@gmail.com>
Subject Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.
Date Tue, 12 Jan 2016 00:52:51 GMT


> On Jan. 11, 2016, 8:45 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line 102
> > <https://reviews.apache.org/r/39803/diff/7/?file=1181320#file1181320line102>
> >
> >     Please do not use a `default` branch when switching over enum values; instead
move the `UNREACHABLE` to the end of the function (you already `return` on all other branches).
> >     
> >     Should we ever add another value `FollowSymlink` the current code would silently
take the unreachable branch and fail at runtime; without the `default` branch modern C++ compilers
like clang or gcc might emit a warning a compile time that not all enum values are covered.
We do pretty consistently follow this pattern elsewhere (this would add just a second instance
of this antipattern, see the dropped issue on this function).

Ok, sorry, it wasn't clear that we wanted to address this in this review.

I'll update the review with a first cut at this problem.


- Alex


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


On Jan. 5, 2016, 12:12 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 12:12 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van Remoortere,
and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Implemented stout/os/stat.hpp`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe

>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp PRE-CREATION

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 5b38b9af654d7d1c574f0cc573083b66693ced1d

>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp d46e262e0fd1c2de36f3bf19d8bd693c23bf58cd

> 
> Diff: https://reviews.apache.org/r/39803/diff/
> 
> 
> Testing
> -------
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


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