-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Update the minimum supported WordPress version to 5.7 #34536
Conversation
@@ -1,463 +0,0 @@ | |||
<?php |
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.
I noticed that this file barely changed since February. So I think the version in Core in 5.7 should be enough. Regardless if it's enough or not we have a check in load.php
that prevents the file from loading in 5.7. So I thought I'd remove but maybe I'm wrong here. cc @noisysocks
I know we have two other endpoints in this situations: widgets and widget types but these two saw a lot of changes since 5.6 So I think we might have an error there in old versions since they're not being loaded from the plugin but the Core one is used which is probably outdated right? Should we instead rename the endpoints classnames in the plugin and override the ones used in Core (we do this for template endpoints already)
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.
The widgets and widgets types endpoint could use some backporting from core too. The ones we ship in the plugin diverged from the ones in 5.8.
As for the future changes, #33810 could save us a lot of headache. For example, lately I was adding a new endpoint to /widget-types
and went for the polyfilling path ( #34230).
Should we instead rename the endpoints classnames in the plugin and override the ones used in Core
Yes, +1 for doing 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.
This endpoint didn't ship in 5.7. We can't remove it until the minimum supported WordPress version is 5.8.
Can we remove any of the block support files in https://github.com/WordPress/gutenberg/tree/trunk/lib/block-supports? Some of them were added in WordPress 5.6, but many got some updates. Is it something that we plan to keep overriding through the plugin like core blocks? |
yeah, that was my thinking, I'm fine removing something if it's stable for a long period though, I'll have to check deeper. |
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 looks good so far 👍🏻
Let's wait for confirmation regarding the class marked for removal.
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.
I may be confused here, but was WP_REST_Sidebars_Controller
shipped in 5.7? I can't find it in git history or by grepping 5.7.2 zip bundle.
Well I saw the |
I think we should also be able to remove these bits:
|
I think some of them (if not all) had some small tweaks in WP 5.8: I mentioned it in #34536 (comment) but it looks like we just need to keep them until the implementation becomes stable or make it easier to modify through filters. |
I checked the diff for the block supports and removed the ones that had only "cosmetic" changes. So they're not needed here anymore. I remove a couple other functions that were not needed anymore as well. |
Per our WP - 2 policy, this PR updates the minimum supported WordPress version for the Gutenberg plugin to WP 5.7.