> On Jan. 5, 2016, 2:56 p.m., Alexander Rojas wrote:
> > Pretty clever code, it would be nice if some of the techniques were at 
> > least explained in a blog post with a link in the code. I'm mostly 
> > concerend about the use of C++17 functions (`std::rank`). Does that mean we 
> > are pumping the minimum required version for the compiler? otherwise, could 
> > we investigate the compiler and/or stdlib support?

Yeah, perhaps we should write a blog post. `std::rank` in specific is a C++11 
feature. Which other ones are you concerned about?


> On Jan. 5, 2016, 2:56 p.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 561
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181766#file1181766line561>
> >
> >     Doesn't the `std::result_of` has the same problem in windows as when 
> > used in `Future`?

Yeah, it does. But since we're blocked on other reviewers for that one, I'm 
planning to follow-up and update whichever one gets committed later.
I think changes are, this one will get committed first and I'll follow-up to 
use `<stout/result_of.hpp>` when it gets committed.


> On Jan. 5, 2016, 2:56 p.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, lines 368-371
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181766#file1181766line368>
> >
> >     I have a couple of questions here:
> >     
> >     1. Why do you use a `U&`.
> >     2. Can you add a comment on how this work, it is definitely not clear 
> > for even an intermediate level C++ programmer.

1. We're checking the conditions for a range-for loop. The `begin` and `end` 
are called on lvalue arguments, rather than r-value arguments (if you're asking 
why not `std::declval<U>()`) and they are not necessarily called on 
const-lvalue arguments (if you're asking why not `std::declval<const U&>()`). 
Having said that, it would have been more accurate for me to use 
`IsSequence<const T>` below at the call-site.
2. Yep, I can add more comments.


- Michael


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


On Jan. 5, 2016, 10:20 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 10:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
>   3rdparty/libprocess/3rdparty/stout/README.md 
> a2a3dc6120b10c8fe82088b9f0ebecfa5642f945 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 98ea47794b3a7c99b3cbd2418ba6e36eb5951259 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 62ad461eb228b688f1ceac16cfb003561ed5a806 
>   3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> bf2a2b8a9f67c6a4cf66b156b9c14fae015a8af0 
> 
> Diff: https://reviews.apache.org/r/41593/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to