mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 30110: Added support for RepeatedPtrField to ::protobuf::write.
Date Thu, 22 Jan 2015 23:51:03 GMT


> On Jan. 22, 2015, 11:32 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 298
> > <https://reviews.apache.org/r/30110/diff/4/?file=829591#file829591line298>
> >
> >     I thought the consensus was to have a internal write in src/slave/state.hpp
(not here)?
> 
> Michael Park wrote:
>     Yes, the `internal::write` that handles `std::string`, `google::protobuf::Message`,
`google::protobuf::RepeatedPtrField<T>`, and `Resources` exist in  https://reviews.apache.org/r/29918.
>     
>     This is an attempt to keep the API separate (`os::write` and `protobuf::write`) while
keeping implementation common since otherwise we'd have duplicate code.
>     All `write(path, T)` can/should use this function (although this is probably not
the correct place to put it). Currently I call this function from `os::write` and `protobuf::write`.
I've kept those separate as we discussed.
>     
>     I hope this is in line with what we agreed, otherwise I can update again.

IC. I would rather not do this change for readability (you need to jump from protobuf.hpp
to os.hpp to see the implementation of write(path, T), and reader might wonder what's the
function pointer in internal::write).

Could you please revert those changes? Thanks a lot!


- Jie


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


On Jan. 22, 2015, 4:52 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30110/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 4:52 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added support for `RepeatedPtrField` to `::protobuf::write`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 62e72b4b16d0f08cce305e27aa7539970263c4e7

>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp b4f5f172d0ea21fbd56dde1eb43d95f9cddad44b

> 
> Diff: https://reviews.apache.org/r/30110/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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