You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2015/08/27 05:38:52 UTC
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Aug. 27, 2015, 3:38 a.m.)
Review request for mesos and Michael Park.
Changes
-------
Extracted Mesos test.
Summary (updated)
-----------------
Added a test for converting JSON arrays to repeated protobufs.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing (updated)
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review97920
-----------------------------------------------------------
Ship it!
Ship It!
- haosdent huang
On Sept. 7, 2015, 9:12 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2015, 9:12 a.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98996
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On Sept. 15, 2015, 7:58 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 7:58 a.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 15, 2015, 7:58 a.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 14, 2015, 10:52 a.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
MPark's comments.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 10, 2015, 6:36 p.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
MPark's comments.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 191-192
> > <https://reviews.apache.org/r/37827/diff/6/?file=1066089#file1066089line191>
> >
> > The above test already performs the roundtrip of Protobuf -> JSON -> Protobuf. Is it beneficial to add an additional conversion to JSON here? What does it further test?
The rationale here is that equality check for protobufs is not well defined (we do it overselves), hence an additional check via converted `JSON` objects looked to me like a reasonable addition.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98371
-----------------------------------------------------------
On Sept. 10, 2015, 6:36 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 6:36 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 191-192
> > <https://reviews.apache.org/r/37827/diff/6/?file=1066089#file1066089line191>
> >
> > The above test already performs the roundtrip of Protobuf -> JSON -> Protobuf. Is it beneficial to add an additional conversion to JSON here? What does it further test?
>
> Alexander Rukletsov wrote:
> The rationale here is that equality check for protobufs is not well defined (we do it overselves), hence an additional check via converted `JSON` objects looked to me like a reasonable addition.
>
> Michael Park wrote:
> Hm, I see. So I think you're saying that if the protobuf happened to parse incorrectly but the test passed due to a bug in `operator==`, converting it back to `JSON` for the final test could expose that bug. Is that correct?
>
> If it is, it doesn't seem like this is the right place to test for that. It would make more sense to me to add a separate test for `operator==` for `SimpleMessage`.
> In general, I think everything being used in a test aside from the thing being tested should have already been tested for correctness.
> Since otherwise we would have to test the dependent functions being used in any given test in addition to the thing that's actually being tested.
>
> What do you think?
I think you're right. Though it's more work, we should do it properly. Will update the RR chain in a while.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98371
-----------------------------------------------------------
On Sept. 10, 2015, 6:36 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 6:36 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, line 37
> > <https://reviews.apache.org/r/37827/diff/6/?file=1066089#file1066089line37>
> >
> > We don't need `inline` here as this is a `cpp` file.
Right, good catch!
> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 44-50
> > <https://reviews.apache.org/r/37827/diff/6/?file=1066089#file1066089line44>
> >
> > How about we use `std::equal` here?
> >
> > ```cpp
> > if (left.id() != right.id() ||
> > left.numbers().size() != right.numbers().size()) {
> > return false;
> > }
> >
> > return std::equal(
> > left.numbers().begin(), left.numbers().end(), right.numbers().begin());
> > ```
Looks like it will work fine with `RepeatedPtrField`. Good suggestion!
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98371
-----------------------------------------------------------
On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2015, 2:46 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Michael Park <mc...@gmail.com>.
> On Sept. 10, 2015, 9:54 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 191-192
> > <https://reviews.apache.org/r/37827/diff/6/?file=1066089#file1066089line191>
> >
> > The above test already performs the roundtrip of Protobuf -> JSON -> Protobuf. Is it beneficial to add an additional conversion to JSON here? What does it further test?
>
> Alexander Rukletsov wrote:
> The rationale here is that equality check for protobufs is not well defined (we do it overselves), hence an additional check via converted `JSON` objects looked to me like a reasonable addition.
Hm, I see. So I think you're saying that if the protobuf happened to parse incorrectly but the test passed due to a bug in `operator==`, converting it back to `JSON` for the final test could expose that bug. Is that correct?
If it is, it doesn't seem like this is the right place to test for that. It would make more sense to me to add a separate test for `operator==` for `SimpleMessage`.
In general, I think everything being used in a test aside from the thing being tested should have already been tested for correctness.
Since otherwise we would have to test the dependent functions being used in any given test in addition to the thing that's actually being tested.
What do you think?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98371
-----------------------------------------------------------
On Sept. 10, 2015, 6:36 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 6:36 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review98371
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 37)
<https://reviews.apache.org/r/37827/#comment154851>
We don't need `inline` here as this is a `cpp` file.
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 44 - 50)
<https://reviews.apache.org/r/37827/#comment154850>
How about we use `std::equal` here?
```cpp
if (left.id() != right.id() ||
left.numbers().size() != right.numbers().size()) {
return false;
}
return std::equal(
left.numbers().begin(), left.numbers().end(), right.numbers().begin());
```
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 166)
<https://reviews.apache.org/r/37827/#comment154853>
`s/message1/message/`?
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 172)
<https://reviews.apache.org/r/37827/#comment154854>
`s/object1/object/`?
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 191 - 192)
<https://reviews.apache.org/r/37827/#comment154855>
The above test already performs the roundtrip of Protobuf -> JSON -> Protobuf. Is it beneficial to add an additional conversion to JSON here? What does it further test?
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto (line 23)
<https://reviews.apache.org/r/37827/#comment154832>
`s/JSON-> conversion/JSON conversion/`
- Michael Park
On Sept. 9, 2015, 2:46 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2015, 2:46 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 9, 2015, 2:46 p.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 7, 2015, 9:12 a.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Sept. 3, 2015, 9:29 p.m., Michael Park wrote:
> > Perhaps a dumb question, but may I ask why you needed to introduce `SimpleMessage`?
I would like to compare protobuf messages with `EXPECT_EQ`, therefore I need an equality operator, which will be too long for the existing `Message` proto.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review97683
-----------------------------------------------------------
On Sept. 3, 2015, 1:33 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2015, 1:33 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review97683
-----------------------------------------------------------
Perhaps a dumb question, but may I ask why you needed to introduce `SimpleMessage`?
- Michael Park
On Sept. 3, 2015, 1:33 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2015, 1:33 p.m.)
>
>
> Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 3, 2015, 1:33 p.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/
-----------------------------------------------------------
(Updated Sept. 1, 2015, 2:25 p.m.)
Review request for mesos, haosdent huang, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Bugs: MESOS-3312
https://issues.apache.org/jira/browse/MESOS-3312
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
Diff: https://reviews.apache.org/r/37827/diff/
Testing
-------
make check (Mac OS 10.10.4)
**NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
Thanks,
Alexander Rukletsov
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 27, 2015, 5:51 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 179-181
> > <https://reviews.apache.org/r/37827/diff/2/?file=1055718#file1055718line179>
> >
> > From the method name, I think it's not completely obvious what the type is.
> >
> > Since I wouldn't want to type `Try<google::protobuf::RepeatedPtrField<T>>` either, you could consider renaming to something like `protobuf::parseRepeated<T>`.
>
> Alexander Rukletsov wrote:
> We can do that. If we rename it to `parseRepeated()` we can even change the signature to take `JSON::Object` as a parameter.
Decided to take another approach here, see previous review in the chain.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review96713
-----------------------------------------------------------
On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 3:38 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 27, 2015, 5:51 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 179-181
> > <https://reviews.apache.org/r/37827/diff/2/?file=1055718#file1055718line179>
> >
> > From the method name, I think it's not completely obvious what the type is.
> >
> > Since I wouldn't want to type `Try<google::protobuf::RepeatedPtrField<T>>` either, you could consider renaming to something like `protobuf::parseRepeated<T>`.
We can do that. If we rename it to `parseRepeated()` we can even change the signature to take `JSON::Object` as a parameter.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review96713
-----------------------------------------------------------
On Aug. 27, 2015, 3:38 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 3:38 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 37827: Added a test for converting JSON arrays to
repeated protobufs.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37827/#review96713
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 179 - 181)
<https://reviews.apache.org/r/37827/#comment152353>
From the method name, I think it's not completely obvious what the type is.
Since I wouldn't want to type `Try<google::protobuf::RepeatedPtrField<T>>` either, you could consider renaming to something like `protobuf::parseRepeated<T>`.
- Joseph Wu
On Aug. 26, 2015, 8:38 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37827/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 8:38 p.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp c56d6a3098293eb3659b3066f10e875927ec3ac3
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h cfc2803e42284f641879fb24bce1282215c8ea52
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc a1d4084661345f9367c75f9db61279f032b93e69
> 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto bbd36d39e9588eb8eea6d739451ad3bab029ca08
>
> Diff: https://reviews.apache.org/r/37827/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.10.4)
>
> **NOTE**: Filed [MESOS-3323](https://issues.apache.org/jira/browse/MESOS-3323) to clean up protobuf generation.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>