Skip to content
New issue

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

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

Already on GitHub? # to your account

Switch away from gogo/status #13

Merged
merged 2 commits into from
Apr 5, 2018
Merged

Conversation

johanbrandhorst
Copy link
Member

@johanbrandhorst johanbrandhorst commented Apr 3, 2018

Ridicolously enough, this now works 🤓.

Fixes #12

@awalterschulze
Copy link
Member

wat! really?

So we can even delete the gogo/status package?

@johanbrandhorst
Copy link
Member Author

As far as I can tell... But the gogo/status package is still necessary if you're working with types that don't define XXX_MessageName or use goproto_registration. That's probably a very narrow use case.

@awalterschulze
Copy link
Member

Should the example.proto maybe then include?

option (gogoproto.messagename_all) = true;

@johanbrandhorst
Copy link
Member Author

Good idea. I briefly played around with removing goproto_registration but it's still necessary because grpc-gateway calls to proto.EnumValueMap when resolving query parameters:

https://github.com/grpc-ecosystem/grpc-gateway/blob/261dafe7abb6a223549d14d02faccffea736b7e8/runtime/query.go

@johanbrandhorst
Copy link
Member Author

Bump :(?

@awalterschulze
Copy link
Member

Interesting. I replied via email a day ago, but it didn't seem to arrive as a comment here. Sorry.

@awalterschulze
Copy link
Member

Good to know about proto.EnumValueMap. Very interesting.

@johanbrandhorst johanbrandhorst merged commit 2cbb435 into master Apr 5, 2018
@johanbrandhorst johanbrandhorst deleted the investigate-grpc-status branch April 5, 2018 20:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants