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