-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix for referenced enums in field params #1453
Fix for referenced enums in field params #1453
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1453 +/- ##
==========================================
- Coverage 54.14% 54.09% -0.05%
==========================================
Files 42 42
Lines 4375 4379 +4
==========================================
Hits 2369 2369
- Misses 1750 1754 +4
Partials 256 256 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Matthew, thanks a lot for this fix! I would appreciate if we could create a test case that fails before this fix is applied. It could just be a matter of adding an imported enum to one of the messages that is used in one of the example proto files in such a way that it would have triggered this bug before the fix. Maybe somewhere in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/proto/examplepb/a_bit_of_everything.proto? See https://github.com/grpc-ecosystem/grpc-gateway/blob/master/CONTRIBUTING.md#i-want-to-regenerate-the-files-after-making-changes for how to regenerate the files after changing them.
Thanks!
Let me know if the tests aren't sufficient. They do recreate the situation I was experiencing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you so much!
Thanks for your contribution! Could you please cherry pick this against the v2 branch as well? |
Fixes #1203
Basically this compares the Enum's source file to the source of the Service instead of the source of the Message. Enum & Message could be from an external source, but Service will always be current source.