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

protoc-gen-go: add convenience function for getting ServiceDescriptorProtos #579

Closed
wants to merge 11 commits into from

Conversation

dfawley
Copy link
Contributor

@dfawley dfawley commented Apr 2, 2018

Like #516, but in the main go code generator instead of the grpc plugin.

@dfawley
Copy link
Contributor Author

dfawley commented Apr 2, 2018

Sorry for the noise. I think I see what's happening now. Because this is a dev branch of protobuf, go get doesn't work with it (it attempts to get from the master branch). Not sure what the best way to deal with this is, but I'll keep trying...

@dfawley
Copy link
Contributor Author

dfawley commented Apr 2, 2018

The last error seems to be a context problem. I'll have to look, but it looks like the grpc plugin on dev is (and was before this PR) importing "context" instead of "x/net/context". This would be a problem, if true.

@dsnet
Copy link
Member

dsnet commented Apr 2, 2018

I've become increasingly convinced that support for accessing ServiceDescriptorProto and MethodDescriptorProto should be in the mainline generator. However, I'm not sure this is the right API for this. It should be thought of as a whole with regard to how all other descriptors are retrieved. See #489 (comment).

@dfawley
Copy link
Contributor Author

dfawley commented Apr 2, 2018

@dsnet, that sounds fine to me.

I'd still like to salvage some sort of tests based on this, in light of the current incompatibility of the grpc plugin with Go 1.6 (please see #580).

@dsnet
Copy link
Member

dsnet commented Jun 6, 2018

Given that #364 (comment) has a more comprehensive plan for adding this functionality, what would you like to salvage from this current PR to add today? The tests you have seem to depend on the functionality that I prefer not adding at the present moment.

@dsnet dsnet changed the title Add convenience function for getting ServiceDescriptorProtos protoc-gen-go: add convenience function for getting ServiceDescriptorProtos Jun 6, 2018
@dfawley
Copy link
Contributor Author

dfawley commented Jun 11, 2018

@dsnet, a simple "nop" test that imports a .pb.go file generated by the grpc plugin would be useful. Unless I'm missing something, I don't think we have any tests that ensure the generated code will even compile.

@dsnet
Copy link
Member

dsnet commented Jun 12, 2018

Done. Are you okay with closing this PR as stale?

@dfawley
Copy link
Contributor Author

dfawley commented Jun 12, 2018

SGTM, thanks.

@dfawley dfawley closed this Jun 12, 2018
@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants