-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
add missing hasOneof method to check presence of oneof fields #8003
add missing hasOneof method to check presence of oneof fields #8003
Conversation
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.
This is great, thanks! Just a few small comments.
* | ||
* @return boolean | ||
*/ | ||
PHP_METHOD(Message, hasOneof) { |
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.
Can you add a test for this?
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.
Test looks great, but it might be better to move it into one of the existing test files (I think GeneratedClassTest.php
would be a good option). Adding new files is a bit more involved because then the new file has to get added to the Makefile:
https://github.com/protocolbuffers/protobuf/blob/master/Makefile.am#L767-L961
I kicked off the tests. If you move your new test into an existing file, it should fix the "distcheck" test failure. |
We'd like to cut a branch for our upcoming release today and this fix should definitely go in. Are you planning to push again in the next few hours? If not I may need to adopt this change in a different PR to get it merged. |
Actually let me merge this now and the send a follow-up PR to fix the test failures. |
Possible solution for #8001 and #7961
test.proto:
test.php:
Output:
$ php ./test.php Fatal error: Uncaught Error: Call to undefined method Test\Test::hasOneof() ...
Expected output: