mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jan Schlicht" <...@mesosphere.io>
Subject Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)
Date Mon, 28 Sep 2015 13:42:38 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message