> On Feb. 27, 2014, 10:52 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/abort_msg.hpp, line 27
> > <https://reviews.apache.org/r/18549/diff/1/?file=505246#file505246line27>
> >
> >     Why ABORT_MSG instead of just ABORT?

I toyed with the idea of having ABORT as a wrapper for abort() for consistency 
but then dropped that. Also, it was called FATAL_MSG at some point because 
FATAL clashed with glog.


> On Feb. 27, 2014, 10:52 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/check.hpp, line 84
> > <https://reviews.apache.org/r/18549/diff/1/?file=505247#file505247line84>
> >
> >     There has always been a bug here: we need to append the error here 
> > rather than down in the constructor otherwise any appended strings via '<<' 
> > will end up being written in between (and be confusing and hard to read). 
> > Just like 'expression', this will let you kill 'error' as an instance 
> > member too!

added file and line here too.


> On Feb. 27, 2014, 10:52 a.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.

so will abort. This means we have no glog dependency in stout.


> On Feb. 27, 2014, 10:52 a.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?

I was just doing a grep/replace for fatal.


- Dominic


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


On Feb. 27, 2014, 11:03 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18549/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 11:03 a.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