-
Notifications
You must be signed in to change notification settings - Fork 2.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
Fix block registration from metadata #3501
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.
Adding some notes regarding the changes done in this PR
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.
Left one minor feedback.
99e2cbe
to
d2f4977
Compare
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
…ix/block-registration
Added PHPUnit tests and fixed issues that surfaced from said tests. |
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.
Thanks for the tests @aristath! I've left a few thoughts in this review. 🙂
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
…ix/block-registration
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
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.
Please consider 0bfc842. This change will revert this change.
Can you please elaborate? How does it revert that change? I don't see it 🤔 |
In 0bfc842, I made it so the file_exists is not run on the core blocks. The file exists is pointless as it always exists and ends up I/O. In this PR, you move the file_exists out the if statement, make it run again always, reverting my change. |
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
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.
Looks good to me.
@aristath Is this good to commit. |
Yes, it is. 👍 |
Thanks for the PR! Merged in r57026. |
Trac ticket: https://core.trac.wordpress.org/ticket/56865
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.