Skip to content

Fix generating method output with nested resource#22594

Merged
apolcyn merged 2 commits intogrpc:masterfrom
Oshiumi:fix-nested-message-output-generation
May 8, 2020
Merged

Fix generating method output with nested resource#22594
apolcyn merged 2 commits intogrpc:masterfrom
Oshiumi:fix-nested-message-output-generation

Conversation

@Oshiumi
Copy link
Copy Markdown
Contributor

@Oshiumi Oshiumi commented Apr 6, 2020

Fixes #21561

The grpc_ruby_plugin protoc plugin has a bug in generating Ruby output for proto files with nested resources and using the ruby_package option.

The plugin works fine when using either nested resources or ruby_package, but not both.
#21561

Fix to use descriptor->full_name() removed package name instead of descriptor->name() when using the ruby_package option.

@apolcyn

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Apr 6, 2020

CLA Check
The committers are authorized under a signed CLA.

@stale
Copy link
Copy Markdown

stale bot commented May 6, 2020

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

Copy link
Copy Markdown
Contributor

@apolcyn apolcyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, I just have some comments about formatting. Please address and then I'll approve

std::string proto_type = descriptor->full_name();
ReplacePrefix(&proto_type, package, ""); // remove the leading package if present
ReplacePrefix(&proto_type, ".", ""); // remove the leading . (no package)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please delete unnecessary blank line, here and below.

inline grpc::string RubyTypeOf(const grpc::protobuf::Descriptor* descriptor,
const grpc::string& package) {
std::string proto_type = descriptor->full_name();
ReplacePrefix(&proto_type, package, ""); // remove the leading package if present
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address clang format suggestion:

-  ReplacePrefix(&proto_type, package, "");  // remove the leading package if present
-  ReplacePrefix(&proto_type, ".", "");      // remove the leading . (no package)
+  ReplacePrefix(&proto_type, package,
+                "");                    // remove the leading package if present
+  ReplacePrefix(&proto_type, ".", "");  // remove the leading . (no package)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition/never stale lang/ruby release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ruby: grpc_ruby_plugin bug using ruby_package and nested resources

4 participants