You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2016/01/04 20:14:54 UTC
Re: Review Request 41593: stout: Added `jsonify` function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 7:14 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Added back the ref-qualifiers.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> 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
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review112845
-----------------------------------------------------------
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?
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (lines 368 - 371)
<https://reviews.apache.org/r/41593/#comment173330>
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.
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (line 561)
<https://reviews.apache.org/r/41593/#comment173332>
Doesn't the `std::result_of` has the same problem in windows as when used in `Future`?
- Alexander Rojas
On Jan. 5, 2016, 11: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, 11: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
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/Makefile.am, line 38
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181763#file1181763line38>
> >
> > Is formatting off or it's just review board?
The formatting is off I think. I can't be sure with all of these tabs all over the place.
> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp, lines 25-26
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181769#file1181769line25>
> >
> > We have a good tradition to prepend each test with a comment explaining the purpose of the test. Though it sometimes obvious without any comments, let's stick to this tradition to avoid "broken windows".
Added comments! and more tests!
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review113230
-----------------------------------------------------------
On Jan. 5, 2016, 10:38 p.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:38 p.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
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, lines 97-99
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181766#file1181766line97>
> >
> > Why this behaviour? I would expect if `set()` is not called, nothing is printed. If a field is optional, I can imagine some cases treating the absence of a value differently to `false`, effectively implementing tribool logic.
> >
> > Or am I understanding the workflow wrong?
>
> Michael Park wrote:
> This behavior is consistent with other writers such as `ArrayWriter` where not calling `append` results in `[]`. The provided expectation is that a writer will always write something. If a field is optional and therefore we want to omit writing under certain conditions, the pattern is to not invoke the `write` at all.
>
> For exmaple,
> ```cpp
> 190 // Omit pid for http frameworks.
> 191 if (framework.pid.isSome()) {
> 192 writer->field("pid", string(framework.pid.get()));
> 193 }
> ```
>
> Does this seem reasonable?
I see. First, almost no-one will use `BooleanWriter` directly, but rather indirectly via `JSON::ObjectWriter::field()`. Second the idea is once we create an object, we should add an entry in the JSON result. No entry — no object. Makes sense, since JSON does not allow empty values for fields AFAIK. Mind extending the example you have in the README with this approach for optional fields?
> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/Makefile.am, line 38
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181763#file1181763line38>
> >
> > Is formatting off or it's just review board?
>
> Michael Park wrote:
> The formatting is off I think. I can't be sure with all of these tabs all over the place.
Yeah, it's a bit odd. In makefiles we align using tabs.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review113230
-----------------------------------------------------------
On Jan. 8, 2016, 4:47 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 4:47 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
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 7, 2016, 10:25 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, lines 97-99
> > <https://reviews.apache.org/r/41593/diff/14/?file=1181766#file1181766line97>
> >
> > Why this behaviour? I would expect if `set()` is not called, nothing is printed. If a field is optional, I can imagine some cases treating the absence of a value differently to `false`, effectively implementing tribool logic.
> >
> > Or am I understanding the workflow wrong?
This behavior is consistent with other writers such as `ArrayWriter` where not calling `append` results in `[]`. The provided expectation is that a writer will always write something. If a field is optional and therefore we want to omit writing under certain conditions, the pattern is to not invoke the `write` at all.
For exmaple,
```cpp
190 // Omit pid for http frameworks.
191 if (framework.pid.isSome()) {
192 writer->field("pid", string(framework.pid.get()));
193 }
```
Does this seem reasonable?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review113230
-----------------------------------------------------------
On Jan. 8, 2016, 4:47 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 4:47 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
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review113230
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/Makefile.am (line 38)
<https://reviews.apache.org/r/41593/#comment173729>
Is formatting off or it's just review board?
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (lines 97 - 99)
<https://reviews.apache.org/r/41593/#comment173731>
Why this behaviour? I would expect if `set()` is not called, nothing is printed. If a field is optional, I can imagine some cases treating the absence of a value differently to `false`, effectively implementing tribool logic.
Or am I understanding the workflow wrong?
3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp (lines 25 - 26)
<https://reviews.apache.org/r/41593/#comment173730>
We have a good tradition to prepend each test with a comment explaining the purpose of the test. Though it sometimes obvious without any comments, let's stick to this tradition to avoid "broken windows".
- Alexander Rukletsov
On Jan. 5, 2016, 10:38 p.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:38 p.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
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 14, 2016, 4:14 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 659
> > <https://reviews.apache.org/r/41593/diff/16/?file=1195136#file1195136line659>
> >
> > Why do you need to pull this variable out?
This is consistent from the original code.
> On Jan. 14, 2016, 4:14 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 637
> > <https://reviews.apache.org/r/41593/diff/16/?file=1195136#file1195136line637>
> >
> > Are you pulling this variable out to optimize away the calls to `field_count()`?
It looks like it. Synced with BenH offline, and we agreed this is ok.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review114398
-----------------------------------------------------------
On Jan. 15, 2016, 4 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 4 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 f32493f90f3bc2a62119dcea49eb5f4966388bf5
> 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
> 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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
>
> Diff: https://reviews.apache.org/r/41593/diff/
>
>
> Testing
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review114398
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/README.md (line 250)
<https://reviews.apache.org/r/41593/#comment175233>
{ on newline.
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 637)
<https://reviews.apache.org/r/41593/#comment175236>
Are you pulling this variable out to optimize away the calls to `field_count()`?
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 659)
<https://reviews.apache.org/r/41593/#comment175235>
Why do you need to pull this variable out?
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 699)
<https://reviews.apache.org/r/41593/#comment175237>
s/str/s/g please.
3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp (line 85)
<https://reviews.apache.org/r/41593/#comment175238>
{ on newline please (below too).
3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp (lines 148 - 150)
<https://reviews.apache.org/r/41593/#comment175239>
:-) !!!!!!!!!!!!!!!!!!!!!!
- Benjamin Hindman
On Jan. 13, 2016, 12:57 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 12:57 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 ec851dcb08d938defeb6066810011e003d594b29
> 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
> 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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
>
> Diff: https://reviews.apache.org/r/41593/diff/
>
>
> Testing
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 15, 2016, 1:26 p.m.)
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-4237
https://issues.apache.org/jira/browse/MESOS-4237
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 f32493f90f3bc2a62119dcea49eb5f4966388bf5
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Added tests to `<stout/jsonify_tests.cpp>`
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 15, 2016, 4 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Addressed some of BenH's comments.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/Makefile.am 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481
3rdparty/libprocess/3rdparty/stout/README.md a2a3dc6120b10c8fe82088b9f0ebecfa5642f945
3rdparty/libprocess/3rdparty/stout/include/Makefile.am f32493f90f3bc2a62119dcea49eb5f4966388bf5
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Added tests to `<stout/jsonify_tests.cpp>`
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 12:57 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Added a note about exception-enabled streams.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/Makefile.am 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481
3rdparty/libprocess/3rdparty/stout/README.md a2a3dc6120b10c8fe82088b9f0ebecfa5642f945
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ec851dcb08d938defeb6066810011e003d594b29
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Added tests to `<stout/jsonify_tests.cpp>`
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 8, 2016, 4:47 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Addressed Alexander and AlexR's comments.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
-------
Added tests to `<stout/jsonify_tests.cpp>`
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 10:38 p.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 (updated)
-------
Added tests to `<stout/jsonify_tests.cpp>`
Thanks,
Michael Park
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
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.
Summary (updated)
-----------------
Added `jsonify` function to stout.
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
Re: Review Request 41593: stout: Added `jsonify` function.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 10:19 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Removed `IsWriteFunction`, added tests.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 5, 2016, 9:28 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 107
> > <https://reviews.apache.org/r/41593/diff/13/?file=1181157#file1181157line107>
> >
> > Here and in all dtrs below: Given that we don't know whether exceptions are enabled for `stream` (this is stout), could we wrap this in a `try` block?, e.g., imagine an underlying `fstream` becoming `bad` because there is no space left on the device.
>
> Michael Park wrote:
> Hm... that's a good point. I'm not particularly worried about it for our use case since we don't use exception-enabled streams but it is an issue in the general case. One approach would be to make it a requirement that the stream not be exception-enabled (We can `CHECK` this on construction). If we were to wrap it in a `try` block, the best we can do is swallow the exception with no way to propagate the information which is generally a bad idea...
>
> Benjamin Bannier wrote:
> AFAICT there is no race-free way to `CHECK` that exceptions on `stream` are disabled, so it seems like one would need to fix this by violently documenting a pre-condition.
>
> Not sure how easy it would be to adjust the existing design, but one could imagine closing the semantic brackets (currently in dtrs) from some `finalize` method (phooey!). If something threw from there users could either catch or just `terminate`, but at least we wouldn't summon UB.
Discussed this with BenH, for now we'll leave a comment about how exception-enabled streams are not allowed. Thanks for bringing this up!
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review112802
-----------------------------------------------------------
On Jan. 13, 2016, 12:57 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 12:57 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 ec851dcb08d938defeb6066810011e003d594b29
> 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 017cef466a30e73af36d72332514a4dbb2beb6bc
> 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 34bb2523ebf3d6d390cbfe1623b89fae0053b712
>
> Diff: https://reviews.apache.org/r/41593/diff/
>
>
> Testing
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Jan. 5, 2016, 10:28 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 107
> > <https://reviews.apache.org/r/41593/diff/13/?file=1181157#file1181157line107>
> >
> > Here and in all dtrs below: Given that we don't know whether exceptions are enabled for `stream` (this is stout), could we wrap this in a `try` block?, e.g., imagine an underlying `fstream` becoming `bad` because there is no space left on the device.
>
> Michael Park wrote:
> Hm... that's a good point. I'm not particularly worried about it for our use case since we don't use exception-enabled streams but it is an issue in the general case. One approach would be to make it a requirement that the stream not be exception-enabled (We can `CHECK` this on construction). If we were to wrap it in a `try` block, the best we can do is swallow the exception with no way to propagate the information which is generally a bad idea...
AFAICT there is no race-free way to `CHECK` that exceptions on `stream` are disabled, so it seems like one would need to fix this by violently documenting a pre-condition.
Not sure how easy it would be to adjust the existing design, but one could imagine closing the semantic brackets (currently in dtrs) from some `finalize` method (phooey!). If something threw from there users could either catch or just `terminate`, but at least we wouldn't summon UB.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review112802
-----------------------------------------------------------
On Jan. 5, 2016, 11:38 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 11:38 p.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
> -------
>
> Added tests to `<stout/jsonify_tests.cpp>`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: Added `jsonify` function to stout.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 5, 2016, 9:28 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 107
> > <https://reviews.apache.org/r/41593/diff/13/?file=1181157#file1181157line107>
> >
> > Here and in all dtrs below: Given that we don't know whether exceptions are enabled for `stream` (this is stout), could we wrap this in a `try` block?, e.g., imagine an underlying `fstream` becoming `bad` because there is no space left on the device.
Hm... that's a good point. I'm not particularly worried about it for our use case since we don't use exception-enabled streams but it is an issue in the general case. One approach would be to make it a requirement that the stream not be exception-enabled (We can `CHECK` this on construction). If we were to wrap it in a `try` block, the best we can do is swallow the exception with no way to propagate the information which is generally a bad idea...
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review112802
-----------------------------------------------------------
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
>
>
Re: Review Request 41593: stout: Added `jsonify` function.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/#review112802
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp (line 107)
<https://reviews.apache.org/r/41593/#comment173298>
Here and in all dtrs below: Given that we don't know whether exceptions are enabled for `stream` (this is stout), could we wrap this in a `try` block?, e.g., imagine an underlying `fstream` becoming `bad` because there is no space left on the device.
- Benjamin Bannier
On Jan. 4, 2016, 11:39 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 11:39 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 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
>
> Diff: https://reviews.apache.org/r/41593/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 41593: stout: Added `jsonify` function.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 11:39 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Moved `void json(const google::protobuf::Message&);` to `<stout/protobuf.hpp>`.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 41593: stout: Added `jsonify` function.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 7:28 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Used a more familiar `friend operator<<` declaration.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 41593: stout: Added `jsonify` function.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41593/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 7:23 p.m.)
Review request for mesos and Benjamin Hindman.
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/41593/diff/
Testing
-------
Thanks,
Michael Park