-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
cmd/protoc-gen-go-grpc: test the embedded struct at registration time for proper usage #7438
Conversation
cmd/protoc-gen-go-grpc/grpc.go
Outdated
g.P("// If the following call pancis, it indicates Unimplemented", serverType, " was embedded by pointer") | ||
g.P("// and is nil. This will cause panics if an unimplemented method is ever invoked, so we test this") | ||
g.P("// at initialization time to prevent it from happening at runtime later due to I/O.") |
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.
Nit: Could these comments also be wrapped at 80-cols. Thanks.
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.
Done
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7438 +/- ##
==========================================
- Coverage 81.44% 81.33% -0.11%
==========================================
Files 352 352
Lines 26919 26919
==========================================
- Hits 21924 21895 -29
- Misses 3804 3822 +18
- Partials 1191 1202 +11 |
… for proper usage (grpc#7438)
… for proper usage (grpc#7438)
relnotes for cmd/protoc-gen-go-grpc:
Register<Service>Server
now panics if theUnimplemented<Service>Server
struct is embedded in a way that would lead to runtime panics if an unimplemented method was called. Users are advised to ensure they are properly embedding theUnimplemented
struct for their service, and to regenerate their proto files to confirm.RELEASE NOTES: none