-
Notifications
You must be signed in to change notification settings - Fork 129
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
Fix PHP error for parameter after optional parameter #1465
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.
Moving the object ID isn't really an option as it provides a bad DX.
Instead I think we should introduce new functions which handle the global meta:
function gp_update_meta( $object_id, $meta_key, $meta_value, $type ) {}
function gp_update_global_meta( $meta_key, $meta_value, $type ) {}
function gp_delete_meta( $object_id, $meta_key, $meta_value, $type ) {}
function gp_delete_global_meta( $meta_key, $meta_value, $type ) {}
Thoughts?
Why not only remove the default value, because you always have to use a value when you call to these functions?
These changes pass the tests. I have just checked in my machine. |
I agree with Jesús, since it's not possible to make use of the default value ( |
I should be able to update the PR for that. |
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.
Thank you! Looks good to me!
@ocean90 is it ok if I dismiss your review, to merge this PR? |
The proposed solution takes a new approach, so this changes requested don't apply.
Fixes #1446
I'm not certain the changes to the unit tests are correct. I was unable to run phpunit according to the instruction for
wp-env
😞