> On Sept. 28, 2015, 11:11 a.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762
> > <https://reviews.apache.org/r/38342/diff/4/?file=1085227#file1085227line762>
> >
> >     Because this code has been changed from a constructor to a function, we 
> > can do better than calling `ABORT` here. We can change the function 
> > signature to return a `Try<Object>` and call `Error` here instead of 
> > `ABORT`. For example see the `write` function in line 55.
> >     
> >     The function signature would change to
> >     `inline Try<Object> protobuf(const google::protobuf::Message& message)`
> 
> Klaus Ma wrote:
>     Got your point :).
>     Just one concern on how to use it? We did not handle its result if failed 
> in `src`; personally, this's a kind of code practice to avoid undefined 
> protobuf field type. Anyway, I'll update accordingly if necessary.

Well, unfortunately this change means that the code that is using this function 
has to be changed to handle the error cases. This means checking if the `Try<>` 
contains a value or an error and react accordingly.


- Jan


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


On Sept. 27, 2015, 3:34 a.m., Klaus Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2015, 3:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
>     https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField<T>` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> -------
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Reply via email to