----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65997/#review199202 -----------------------------------------------------------
src/cmake/MesosProtobuf.cmake Line 24 (original), 24 (patched) <https://reviews.apache.org/r/65997/#comment279494> Nit: s/support/supported src/cmake/MesosProtobuf.cmake Line 29 (original), 37 (patched) <https://reviews.apache.org/r/65997/#comment279495> Nit: s/if s/S src/cmake/MesosProtobuf.cmake Lines 51 (patched) <https://reviews.apache.org/r/65997/#comment279496> Same nit src/cmake/MesosProtobuf.cmake Lines 94 (patched) <https://reviews.apache.org/r/65997/#comment279497> We put CPP_OUT files under an build/include/lib? src/cmake/MesosProtobuf.cmake Lines 72-74 (original), 125-132 (patched) <https://reviews.apache.org/r/65997/#comment279498> Yeah! This looks reasonable. src/cmake/MesosProtobuf.cmake Lines 178-187 (patched) <https://reviews.apache.org/r/65997/#comment279500> I had previously added the custom mkdir targets to another file, and just added a dependency here. We should at least add a TODO to bring those here too (it seems to be a more appropriate location), and maybe a TODO to find a better way to ensure the directories are created. This always seemed hacky to me. - Andrew Schwartzmeyer On March 13, 2018, 7:56 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65997/ > ----------------------------------------------------------- > > (Updated March 13, 2018, 7:56 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and > Joseph Wu. > > > Bugs: MESOS-8657 > https://issues.apache.org/jira/browse/MESOS-8657 > > > Repository: mesos > > > Description > ------- > > PROTOC_GENERATE now has the following new features: > (1) With the `LIB` option, compile .proto files found in a third-party > specification library. > (2) Provides the `GRPC` option that, once we support gRPC in cmake, > will generate the `.grpc.pb.h` and `.grpc.pb.cc` files. > (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH` > or `INTERNAL_PROTO_PATH` the fully qualified path to the library's > include directory, and append to list variable > `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR` > the fully qualified path to the directory where the generated > `.pb.h` files are placed. > > > Diffs > ----- > > src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef > src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 > > > Diff: https://reviews.apache.org/r/65997/diff/3/ > > > Testing > ------- > > `make check` with cmake. > > > Thanks, > > Chun-Hung Hsiao > >
