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

Plugin: Guard code from redeclaration errors after WP core merge #39888

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Mar 30, 2022

What?

Part of #39889.

It is expected to declare functions and classes only when they are missing in WordPress core. As soon as we backport the files updated in this PR to WordPress core, they will work only as a fallback for older versions of WordPress.

Why?

To avoid fatal errors because of redeclared functions and classes.

How?

I followed the guidelines at https://github.com/WordPress/gutenberg/tree/trunk/lib#wrap-functions-and-classes-with--function_exists-and--class_exists.

Testing Instructions

There are unit tests that cover those files and they should continue to pass.

We should also test that webfonts API and block patterns continue to work in the editor.

@gziolo gziolo added Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Code Quality Issues or PRs that relate to code quality labels Mar 30, 2022
@gziolo gziolo self-assigned this Mar 30, 2022
@gziolo
Copy link
Member Author

gziolo commented Mar 30, 2022

@jsnajdr – have you started the process of backporting changes related to block patterns to WordPress core?

@aristath – have you started the process of backporting changes related to web fonts API to WordPress core?

@gziolo
Copy link
Member Author

gziolo commented Mar 30, 2022

I opened #39889 to coordinate the backporting tasks.

@gziolo
Copy link
Member Author

gziolo commented Mar 31, 2022

To make the review process it's possible to check the diff with all the whitespace changes removed:

https://github.com/WordPress/gutenberg/pull/39888/files?diff=unified&w=1

Copy link
Contributor

@adamziel adamziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ this is great, thank you! The E2E failure seems to be unrelated, I restarted that check.

@jsnajdr
Copy link
Member

jsnajdr commented Mar 31, 2022

have you started the process of backporting changes related to block patterns to WordPress core?

No I haven't yet, I guess I'm supposed to submit a Core patch that adds the block patterns endpoints? They are currently in the __experimental namespace. Is that OK for Core or should they graduate to wp/v2 as part of the process? The endpoints are quite straightforward and stable, hard to think of any radical change they would potentially need.

@gziolo
Copy link
Member Author

gziolo commented Mar 31, 2022

❤️ this is great, thank you! The E2E failure seems to be unrelated, I restarted that check.

Yes, the same issue with tests as in #39904 (comment). Merged PR passed all checks in trunk 🤷🏻

@gziolo gziolo merged commit 27b4e83 into trunk Mar 31, 2022
@gziolo gziolo deleted the update/guard-declariations-fatal-error branch March 31, 2022 13:14
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Mar 31, 2022
@aristath
Copy link
Member

aristath commented Apr 1, 2022

@aristath – have you started the process of backporting changes related to web fonts API to WordPress core?

Not yet... I'm still unclear about whether we'll merge this API in 6.0, or not, because there are a few PRs related to the webfonts API from folks, so I'm waiting to see if there is a consensus for them and what will happen 👍

@gziolo
Copy link
Member Author

gziolo commented Apr 1, 2022

have you started the process of backporting changes related to block patterns to WordPress core?

No I haven't yet, I guess I'm supposed to submit a Core patch that adds the block patterns endpoints? They are currently in the __experimental namespace. Is that OK for Core or should they graduate to wp/v2 as part of the process? The endpoints are quite straightforward and stable, hard to think of any radical change they would potentially need.

It looks like @hellofromtonya started looking into bringing those endpoints to WordPress core in WordPress/wordpress-develop#2488. I see files listed in the description but no code changes so far. I'm sure the __experimental namespace will get removed as part of the process.

@aristath – have you started the process of backporting changes related to web fonts API to WordPress core?
Not yet... I'm still unclear about whether we'll merge this API in 6.0, or not, because there are a few PRs related to the webfonts API from folks, so I'm waiting to see if there is a consensus for them and what will happen 👍

Thank you @aristath, I will keep an eye on the webfonts API 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Code Quality Issues or PRs that relate to code quality
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants