> On March 13, 2018, 6:54 p.m., Andrew Schwartzmeyer wrote: > > src/cmake/MesosProtobuf.cmake > > Lines 125-163 (patched) > > <https://reviews.apache.org/r/65997/diff/2/?file=1973227#file1973227line125> > > > > Most of this function looks duplicated from `function(PROTOC_GENERATE)` > > but with additional GRPC logic. Can they be combined or refactored? Looking > > at it, it seems we could add the same options instead to > > `function(PROTOC_GENERATE)` and re-use it. Is there anything that > > `function(PROTOC_GENERATE)` does which `function(PROTOC_SPEC_GENERATE)` > > _shouldn't_ do, and can that be conditionalized? > > > > I'm not saying this is a great idea; it may not make sense to share the > > code. But let's take a look.
Let me come up with another prototype that combines the two and we can see if it looks cleaner. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65997/#review199104 ----------------------------------------------------------- On March 9, 2018, 10:44 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65997/ > ----------------------------------------------------------- > > (Updated March 9, 2018, 10:44 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_SPEC_GENERATE is a convenience function that will: > (1) Compile .proto files found in the third-party specification > library under its include directory and place the generated files > in the build folder, under the `include/` directory, or with the > option `INTERNAL` the `src/` directory. > (2) Append to list variables `PUBLIC_PROTO_PATH` or > `INTERNAL_PROTO_PATH` the fully qualified path to the library's > include directory depending on options passed in. > (3) Append to list variables `PUBLIC_PROTOBUF_INCLUDE_DIR` or > `INTERNAL_PROTOBUF_INCLUDE_DIR` (depending on options passed in) > the fully qualified path to the directory where the files are > generated. > (4) Append to list variables `PUBLIC_PROTOBUF_SRC` or > `INTERNAL_PROTOBUF_SRC` (depending on options passed in) the fully > qualified path to the generated .cc file. > Where the exports in (2)(3)(4) are *side effects* and modifies the > variables in the parent scope. > > > Diffs > ----- > > src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef > src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 > > > Diff: https://reviews.apache.org/r/65997/diff/2/ > > > Testing > ------- > > `make check` with cmake. > > > Thanks, > > Chun-Hung Hsiao > >
