Skip to content
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.

Sign up for GitHub

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

Jump to bottom

protoc-gen-go-grpc: Update README.md file #6349

Merged
merged 1 commit into from
Jun 8, 2023
Merged

protoc-gen-go-grpc: Update README.md file #6349

merged 1 commit into from
Jun 8, 2023

Conversation

keita127
Copy link
Contributor

@keita127 keita127 commented Jun 5, 2023 •
edited by zasweq
Loading

I have found these two previous PRs (#4538 #5158) and have also confirmed that the following code actually works.

protoc --go-grpc_out=require_unimplemented_servers=false:. \ foo.proto

However, since go-grpc_out and go-grpc_opt are written separately as the protoc command options on the quick start
guide, isn't this easier to understand?

RELEASE NOTES: N/A

@linux-foundation-easycla
Copy link

CLA Missing ID CLA Not Signed

@keita127 keita127 force-pushed the update-readme-protoc branch from 013cbf5 to 10d61f7 Compare June 5, 2023 16:27
@zasweq
Copy link
Contributor

zasweq commented Jun 6, 2023

Hello, can you please sign the CLA before we can take a look.

@keita127
Copy link
Contributor Author

keita127 commented Jun 7, 2023

/easycla

@keita127
Copy link
Contributor Author

keita127 commented Jun 7, 2023 •
edited
Loading

Thanks for checking. However, I believe I have already signed the CLA. Please let me know if I am wrong.
Also, since tests (tests, 1.20, arm64) have failed, could you please re-run it? I don't think this is caused by my change since I only changed the README.

@zasweq zasweq added this to the 1.57 Release milestone Jun 7, 2023
zasweq
zasweq approved these changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

These are functionally equivalent. I guess since the documentation you point to does look closer to your change, I'll go ahead and approve.

@zasweq zasweq requested a review from dfawley June 8, 2023 04:04
@zasweq zasweq assigned dfawley and unassigned keita127 Jun 8, 2023
dfawley
dfawley approved these changes Jun 8, 2023
View reviewed changes
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Honestly we might want to delete this whole section before too long. It's been years since we deprecated the old code generator, and this option is not recommended for any new use.

@dfawley dfawley merged commit 7a7caf3 into grpc:master Jun 8, 2023
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zasweq zasweq zasweq approved these changes

@dfawley dfawley dfawley approved these changes

Assignees

@dfawley dfawley

Labels
Status: Requires Reporter Clarification Type: Documentation Documentation or examples
Projects
None yet
Milestone
1.57 Release
Development

Successfully merging this pull request may close these issues.
3 participants
@keita127 @zasweq @dfawley