-
Notifications
You must be signed in to change notification settings - Fork 47
Sync changes applied to WordPress 6.9 beta 3 #137
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
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #137 +/- ##
=============================================
- Coverage 90.47% 80.39% -10.09%
- Complexity 177 178 +1
=============================================
Files 20 20
Lines 1470 1474 +4
Branches 120 120
=============================================
- Hits 1330 1185 -145
- Misses 140 289 +149
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @return bool|WP_Error Whether the ability has the necessary permission. | ||
| */ | ||
| public function check_permissions( $input = null ) { | ||
| if ( ! is_callable( $this->permission_callback ) ) { |
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.
Is this redundant/contrary to the behavior in ::prepare_properties()?
( I see this was added in https://github.com/WordPress/wordpress-develop/pull/10431/files, but I can't find the discussion that prompted it. )
IMO the fatal error thrown from ::invoke_callback() is desired behavior, instead of hiding it inside a WP_Error. This error can only happen if a user is extending this class and overloading both the ::$permission_callback property type and either ::prepare_properties() or the constructor to avoid calling ::prepare_proerties() altogether, so the immediacy provides better DX.
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.
It might be redundant, but it doesn't break anything. Feel free to open a patch against WordPress trunk proposing the removal of both checks. There is still time to adjust that logic before the final release.
As for this PR, as noted in the description, the code is copied from WordPress core as is, without modifications.
jonathanbossenger
left a comment
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.
From the inline documentation side, looks good to me
This PR copies all the recent changes applied in WordPress core in preparation for the 6.9 beta 3 / rc 1 releases.
Related commits: