----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67156/#review203320 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/libprocess/include/process/grpc.hpp Lines 37-40 (original), 37-40 (patched) <https://reviews.apache.org/r/67156/#comment285428> Nit: You could drop the parens for symmetry with `client::Runtime` and adjust punctuation, // defines two wrapper classes: `client::Connection` which represents a // connection to a gRPC server, and `client::Runtime` which integrates an event // loop waiting for gRPC responses and provides the `call` interface to create // an asynchronous gRPC call and returns a `Future`. 3rdparty/libprocess/include/process/grpc.hpp Lines 92 (patched) <https://reviews.apache.org/r/67156/#comment285429> Maybe expand while we are at it? * All `Connection` copies share the same gRPC channel. It might make sense to mention that `::grpc::Channel` is thread safe so there is no need to manage concurrent access. 3rdparty/libprocess/include/process/grpc.hpp Lines 101 (patched) <https://reviews.apache.org/r/67156/#comment285433> Taking the `shared_ptr` by `const` ref here while taking ownership seems to be inspired by `::grpc::CreateChannel` itself (where it probably makes a copy) so we might just do the same, but it is weird smart pointer usage. No need to do anything. - Benjamin Bannier On May 16, 2018, 9:25 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67156/ > ----------------------------------------------------------- > > (Updated May 16, 2018, 9:25 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and > Zhitao Li. > > > Bugs: MESOS-8924 > https://issues.apache.org/jira/browse/MESOS-8924 > > > Repository: mesos > > > Description > ------- > > This renaming is made to avoid name conflicts between `::grpc::Channel` > and libprocess' own wrapper. Also, since this wrapper is only used > at the client side, it is moved into the `client` namespace. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/grpc.hpp > 321a46e19c69eafb24012bcef68bb8b0cc6aa436 > 3rdparty/libprocess/src/tests/grpc_tests.cpp > 38cd6c61b54518a1019bb11a3551be13026c3f0d > > > Diff: https://reviews.apache.org/r/67156/diff/3/ > > > Testing > ------- > > make check in libprocess > > NOTE: Mesos cannot be built with this patch standalone. The tests are done > later in the chain. > > > Thanks, > > Chun-Hung Hsiao > >
