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

Ship it!



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 344)
<https://reviews.apache.org/r/41943/#comment174658>

    How about ParseJSONNull?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 349)
<https://reviews.apache.org/r/41943/#comment174660>

    Why "message1" and "message2" for the values of 'str'? How about we set 
these to "value" to keep it simple?
    
    Also, it looks like we should probably compare equality via 
SerializeAsString, rather than checking only one field?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 359)
<https://reviews.apache.org/r/41943/#comment174662>

    You can use the '`->`' operator rather than '`.get().`' Ditto elsewhere.



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (line 380)
<https://reviews.apache.org/r/41943/#comment174661>

    Seems that you don't need to set the optional str here, can we omit it?



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 386 - 387)
<https://reviews.apache.org/r/41943/#comment174659>

    This can be an EXPECT and no need to store the 'parse' variable:
    
    ```
    ASSERT_ERROR(protobuf::parse<tests::Nested>(json.get()););
    ```


- Ben Mahler


On Jan. 11, 2016, 11:40 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41943/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 11:40 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
> -------
> 
> Added test case for stout protobuf parse containing JSON null.
> 
> 
> Diffs
> -----
> 
>   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
> 
>

Reply via email to