Support --descriptor_set_out / --include_imports#15
Support --descriptor_set_out / --include_imports#15binkley wants to merge 13 commits intodtrott:masterfrom
Conversation
…ks if descriptor set is named *.proto.bin.
…ks if descriptor set is named *.proto.bin.
…duction protos without special effort.
…duction protos without special effort.
…duction protos without special effort.
…ferences immediately.
There was a problem hiding this comment.
I think the whole if{} block should be moved inside buildProtocCommand() method for consistency
|
Adding target/classes is probably unnecessary, I think the only reason you have to do it is because the original plugin incorrectly attaches "namespaced" protos to the resources. Effectively, it flattens the nested directory structure for source protos. I was going to fix it on my branch, I had to disable a bunch of integration tests that failed because of that plugin bug. Warning: it may be a potentially breaking change, but I hope nobody out there is relying on the incorrect behaviour. UPDATE: It looks like it's indeed a separate problem. I created a couple of integration tests to see if compile scoped protos can be included into test scoped protos. Apparently we'll need to add some sort of extension point to cater for this scenario in the protoc plugin. I'd like to research if compile output paths are already provided by Maven project in some way. If they are, it would be better to use the internal mechanisms instead of injecting target/classes via a property. |
|
A general comment is the same as on the pull request #13. I propose that we split out mojos for generating non-java files. This will allow for cleaner separation, flexible configuration and sensible defaults. The plugin users will have to define additional plugin executions if needed, which is more in a "Maven way" of doing things. Basic configuration can still be shared between mojos and their respective executions. I think it is great that more people started to contribute to the plugin, but we need to make sure that we are not going to create a mess out of it by trying to cater for all possible usage scenarios in one place. |
I've added support for --descriptor_set_out and --include_imports, among other small changes. Please see my README for details. I made as minimal changes as possible.