----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41943/#review113169 -----------------------------------------------------------
The change to `protobuf.hpp` LGTM. Just a few test nits: 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 344) <https://reviews.apache.org/r/41943/#comment173668> You previously also had a test for a non-nested, but optional null string. Why was it removed? i.e. ``` { "str": null, "json_null": [ "a", "b" ] } ``` 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 346) <https://reviews.apache.org/r/41943/#comment173666> The `strings::remove` isn't necessary since you aren't doing a round-trip string comparison. (i.e. `EXPECT_EQ(message, stringify(parse.get()));`) 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 359) <https://reviews.apache.org/r/41943/#comment173667> Consider comparing to something like: ``` "{ \"str\": \"message\" }" ``` - Joseph Wu On Jan. 6, 2016, 6:07 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41943/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2016, 6:07 p.m.) > > > Review request for mesos, Ben Mahler, Artem Harutyunyan, Jie Yu, Joris Van > Remoortere, Joseph Wu, and Timothy Chen. > > > Bugs: MESOS-4294 > https://issues.apache.org/jira/browse/MESOS-4294 > > > Repository: mesos > > > Description > ------- > > Fixed stout protobuf::parse to support parsing JSON object containing > JSON::Null. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp > 98ea47794b3a7c99b3cbd2418ba6e36eb5951259 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp > bf2a2b8a9f67c6a4cf66b156b9c14fae015a8af0 > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h > 08793c9d21d4b9b7dd3081cfa35afa47a7e0d28a > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc > 797b3b000f3c3fea42cabc3b40baf7235eab6b1e > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto > 4c11e23f875b75bcb9d5c65ee66d55b6f72e546d > > Diff: https://reviews.apache.org/r/41943/diff/ > > > Testing > ------- > > make check (tested on both ubuntu14.04 and OSX10.10) > > > Thanks, > > Gilbert Song > >
