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

Reply via email to