Skip to content

Include GRPC::GenericService from root namespace#25153

Merged
apolcyn merged 1 commit intogrpc:masterfrom
ob-stripe:ob-grpc-service-root
Mar 18, 2021
Merged

Include GRPC::GenericService from root namespace#25153
apolcyn merged 1 commit intogrpc:masterfrom
ob-stripe:ob-grpc-service-root

Conversation

@ob-stripe
Copy link
Copy Markdown
Contributor

@ob-stripe ob-stripe commented Jan 14, 2021

@markdroth

This PR modifies the Ruby service generator to include the GRPC::GenericService module from the root namespace, i.e. it replaces:

include GRPC::GenericService

with:

include ::GRPC::GenericService

This is useful because otherwise, if you already have a submodule named GRPC in any of the class' parent modules, Ruby will use it instead of the grpc gem, e.g.:

module MyApp
  module GRPC
    # presumably some GRPC related things in here
  end
end

# Generated by the protocol buffer compiler.
module MyApp
  module MyProtoService
    class Service
      include GRPC::GenericService # Ruby thinks this means MyApp::GRPC::GenericService
    end
  end
end

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 14, 2021 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@apolcyn apolcyn added lang/ruby release notes: yes Indicates if PR needs to be in release notes kokoro:force-run labels Jan 14, 2021
@ob-stripe ob-stripe force-pushed the ob-grpc-service-root branch from be641d4 to 66f87bc Compare March 1, 2021 20:04
@ob-stripe
Copy link
Copy Markdown
Contributor Author

ob-stripe commented Mar 2, 2021

@apolcyn Any chance this could be merged soon? This issue forces us to do some annoying post-processing steps atm.

@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Mar 17, 2021

How were the edits to _pb.rb files done? Were they performed manually? I'd prefer to avoid them if so.

In general, can we remove all changes in this PR except for the change to src/compiler/ruby_generator.cc?

@ob-stripe
Copy link
Copy Markdown
Contributor Author

ob-stripe commented Mar 17, 2021

Yeah, I think I updated everything with a codemod.

I'll update the PR 👍

@ob-stripe ob-stripe force-pushed the ob-grpc-service-root branch from 66f87bc to 5c08c7d Compare March 17, 2021 16:31
@ob-stripe ob-stripe force-pushed the ob-grpc-service-root branch from 5c08c7d to 25e2b24 Compare March 17, 2021 16:34
@apolcyn apolcyn assigned stanley-cheung and apolcyn and unassigned apolcyn Mar 18, 2021
@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Mar 18, 2021

Adding @stanley-cheung for a second reviewer

@apolcyn
Copy link
Copy Markdown
Contributor

apolcyn commented Mar 18, 2021

Bazel C/C++ Debug Macos shows "tool failure", and logs show "Target is still processing. Please check back later". However, this seems unrelated to this PR, since e.g. Macos Opt tests passed.

@apolcyn apolcyn merged commit 689b62e into grpc:master Mar 18, 2021
@ob-stripe ob-stripe deleted the ob-grpc-service-root branch March 22, 2021 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apolcyn apolcyn apolcyn approved these changes

@stanley-cheung stanley-cheung stanley-cheung approved these changes

Assignees

@apolcyn apolcyn

@stanley-cheung stanley-cheung

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ob-stripe @apolcyn @stanley-cheung @grpc-kokoro