> On Feb. 27, 2014, 6:52 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/check.hpp, lines 89-90
> > <https://reviews.apache.org/r/18549/diff/1/?file=505247#file505247line89>
> >
> >     I'd prefer to keep the google::LogMessageFatal here to mimic the 
> > semantics of CHECK. In particular, the LogMessageFatal will include a stack 
> > trace.
> 
> Dominic Hamon wrote:
>     so will abort. This means we have no glog dependency in stout.

I'd like to preserve the CHECK semantics and do a google::LogMessageFatal. It's 
okay to have a glog dependency here since this can really be seen as an 
extension to glog, just like the gtest.h and gmock.h (in libprocess) are 
extensions. Also, as long as we're using google::LogMessageFatal, how about 
calling it CheckFatal instead of CheckAbort? Thanks!


> On Feb. 27, 2014, 6:52 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines 
> > 49-50
> > <https://reviews.apache.org/r/18549/diff/1/?file=505248#file505248line49>
> >
> >     Why not use UNREACHABLE here?
> 
> Dominic Hamon wrote:
>     I was just doing a grep/replace for fatal.

You can just 'return UNREACHABLE();' here and remove the 'return -1;'.


- Benjamin


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


On Feb. 27, 2014, 7:08 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18549/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 7:08 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1041
>     https://issues.apache.org/jira/browse/MESOS-1041
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5d5a76021900bf27113fe5b730cfd2b6a537d132 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/check.hpp 
> 695db20cb1acff036d8d493be4ff4b5e002fdd90 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 
> 3350929686e5365225f72f48d7c99b42c57fd1ad 
> 
> Diff: https://reviews.apache.org/r/18549/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to