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.

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

[python-runtime] Add MethodDescriptor.CopyToProto() #8327

Merged

Conversation

alkasm
Copy link
Contributor

@alkasm alkasm commented Feb 22, 2021

Fixes: #8326

@google-cla google-cla bot added the cla: yes label Feb 22, 2021
@alkasm
Copy link
Contributor Author

alkasm commented Apr 3, 2021

Bumping this, would like to get it merged in!

@fowles
Copy link
Member

fowles commented Apr 3, 2021

Please address the Mergable failures about release note and language label

@alkasm
Copy link
Contributor Author

alkasm commented Apr 3, 2021

@fowles thanks for taking a look. AFAIK I cannot edit the labels.

@fowles
Copy link
Member

fowles commented Apr 4, 2021

Sorry, didn't realize that you had to be in the official club to edit labels. I will do a final review on Monday.

@fowles
Copy link
Member

fowles commented Apr 6, 2021

Overall this change looks good. Can you add a test in google/protobuf/internal/descriptor_test.py?

@alkasm
Copy link
Contributor Author

alkasm commented Apr 7, 2021

Oh, excellent---the unit test is already there, just skipped for the Python impl. I just removed the skip decorator.

@fowles
Copy link
Member

fowles commented Apr 7, 2021

Thanks! I just kicked off the CI system to test this and will try to merge this later today or early tomorrow.

@fowles fowles merged commit c214856 into protocolbuffers:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python runtime MethodDescriptor is missing a CopyToProto() method
4 participants