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