Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Justify the protobuf/testProtobuf dependency feature or remove it #25

Closed
zhangkun83 opened this issue Jul 9, 2015 · 3 comments
Closed

Comments

@zhangkun83
Copy link
Collaborator

The Protobuf dependencies, i.e.., protobuf, testProtobuf etc, define packages of proto files that will be compiled along with those under the src dir. This feature is inherited from @aantono's plugin. However, I don't feel it should be called dependency. It's just another form of sources. The scenarios described in #15 and #8 sound more like dependency.

Maybe we should move this feature into sourceSets.

@aantono @ejona86 WDYT?

@ejona86
Copy link
Collaborator

ejona86 commented Jul 9, 2015

SGTM. Since true dependencies would be passed to protoc with -I we will need to make it more obvious which files will be code generated and which will just be used for compilation.

@aantono
Copy link
Contributor

aantono commented Jul 9, 2015

The reason those are called dependencies (I refer to protobuf and testProtobuf and whatever other sources you have) is because they truly are archive files, jar, zip, tar, etc, which contain .proto files that are shared.
While we definitely DO NOT want to share pre-compiled Java classes, as it causes all kinds of runtime linking errors if versions of Google's Protobuf Library don't match, but the usecase of sharing the actual protos is very valid. At my place of employment, we have a shared domain model dictionary in a form of proto files, which we version and have checked into Artifactory as a tarball. So for us it is definitely a dependency that looks like protobuf group: 'com.*.protos', name: 'protos-data', version: '1.+', ext: 'tar.gz'. That does not prevent you to have a source set for locally residing proto files, which the plugin also supports, to be living in src/main/protos, and if the files are there, they will automatically be included.

It seems like the issue is really between what do we pass as files for protoc to compile vs. which files get passed in as -I arguments. But I would definitely NOT remove the support of being able to declare protobuf file archives as build artifact dependencies.

Also, see comments in #8 (comment)

Thoughts?

@zhangkun83
Copy link
Collaborator Author

As far as I can tell, this feature has been well used by API services that publish protos as artifacts. We can spare it for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants