You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/05/09 01:23:16 UTC
Review Request 21246: Fixed Protobuf to JSON serialization of optional
fields with default values.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21246/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
-------
Previously, only optional fields which where explicitly set would be serialized regardless whether default values was present.
This patch adds those fields explicitly.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ddc13e9
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp d1a58fc
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 75a3919
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 169e177
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 2c38489
Diff: https://reviews.apache.org/r/21246/diff/
Testing
-------
Protobuf test case has been expanded with
1) Optional field _without_ default value -> should not be serialized.
2) Optional field _with_ default value -> *should* be serialized
make check
Thanks,
Niklas Nielsen
Re: Review Request 21246: Fixed Protobuf to JSON serialization of optional
fields with default values.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21246/#review42557
-----------------------------------------------------------
Bad patch!
Reviews applied: [21246]
Failed command: git apply --index 21246.patch
Error:
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp:473
error: 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp: patch does not apply
error: patch failed: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp:73
error: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp: patch does not apply
error: patch failed: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h:480
error: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h: patch does not apply
error: patch failed: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc:53
error: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc: patch does not apply
error: patch failed: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto:63
error: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto: patch does not apply
- Mesos ReviewBot
On May 9, 2014, 12:26 a.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21246/
> -----------------------------------------------------------
>
> (Updated May 9, 2014, 12:26 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Previously, only optional fields which where explicitly set would be serialized regardless whether default values was present.
> This patch adds those fields explicitly.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ddc13e9
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp d1a58fc
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 75a3919
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 169e177
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 2c38489
>
> Diff: https://reviews.apache.org/r/21246/diff/
>
>
> Testing
> -------
>
> Protobuf test case has been expanded with
> 1) Optional field _without_ default value -> should not be serialized.
> 2) Optional field _with_ default value -> *should* be serialized
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 21246: Fixed Protobuf to JSON serialization of optional
fields with default values.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21246/
-----------------------------------------------------------
(Updated May 8, 2014, 5:26 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Addressed Ben's comments.
Repository: mesos-git
Description
-------
Previously, only optional fields which where explicitly set would be serialized regardless whether default values was present.
This patch adds those fields explicitly.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ddc13e9
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp d1a58fc
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 75a3919
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 169e177
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 2c38489
Diff: https://reviews.apache.org/r/21246/diff/
Testing
-------
Protobuf test case has been expanded with
1) Optional field _without_ default value -> should not be serialized.
2) Optional field _with_ default value -> *should* be serialized
make check
Thanks,
Niklas Nielsen
Re: Review Request 21246: Fixed Protobuf to JSON serialization of optional
fields with default values.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21246/#review42554
-----------------------------------------------------------
Ship it!
Yeeessssss.
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
<https://reviews.apache.org/r/21246/#comment76362>
Can you add a comment here that just explains that we first look through all the possible fields to determine both the set fields and the optional fields with a default that are not set? For posterity it would be nice to mention why ListFields was insufficient.
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
<https://reviews.apache.org/r/21246/#comment76361>
Weird indentation here, should be indented up to the '(', see http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Boolean_Expressions#Boolean_Expressions.
- Benjamin Hindman
On May 8, 2014, 11:29 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21246/
> -----------------------------------------------------------
>
> (Updated May 8, 2014, 11:29 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Previously, only optional fields which where explicitly set would be serialized regardless whether default values was present.
> This patch adds those fields explicitly.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ddc13e9
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp d1a58fc
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 75a3919
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 169e177
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 2c38489
>
> Diff: https://reviews.apache.org/r/21246/diff/
>
>
> Testing
> -------
>
> Protobuf test case has been expanded with
> 1) Optional field _without_ default value -> should not be serialized.
> 2) Optional field _with_ default value -> *should* be serialized
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 21246: Fixed Protobuf to JSON serialization of optional
fields with default values.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21246/
-----------------------------------------------------------
(Updated May 8, 2014, 4:29 p.m.)
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
-------
Previously, only optional fields which where explicitly set would be serialized regardless whether default values was present.
This patch adds those fields explicitly.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ddc13e9
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp d1a58fc
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 75a3919
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 169e177
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 2c38489
Diff: https://reviews.apache.org/r/21246/diff/
Testing
-------
Protobuf test case has been expanded with
1) Optional field _without_ default value -> should not be serialized.
2) Optional field _with_ default value -> *should* be serialized
make check
Thanks,
Niklas Nielsen