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.

Sign up for GitHub

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

Jump to bottom

Fix WP_Term::$data dynamic property #6426

Open
wants to merge 35 commits into
base: trunk
Choose a base branch
from
Open

Fix WP_Term::$data dynamic property #6426

wants to merge 35 commits into from
+398 −11

Conversation

anton-vlasenko
Copy link

@anton-vlasenko anton-vlasenko commented Apr 22, 2024 •
edited
Loading

Trac ticket: https://core.trac.wordpress.org/ticket/58087

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.
@github-actions GitHub Actions
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.
@anton-vlasenko anton-vlasenko marked this pull request as ready for review April 23, 2024 16:55
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 23, 2024 •
edited
Loading

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @rockitround.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props antonvlasenko.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
@anton-vlasenko
Copy link
Author

anton-vlasenko commented Apr 23, 2024 •
edited
Loading

This PR needs additional investigation/research since the unit tests are failing.

I think the PR is in good shape now and can be reviewed. I will add the reasoning to the trac ticket soon.
@rockitround

This comment was marked as spam.

Sign in to view
@anton-vlasenko
Handle the dynamic $data property in WP_Term according to the pro…
8bce95d 
…posal by @hellofromtonya: https://core.trac.wordpress.org/ticket/60875.
@anton-vlasenko
Update the docblock.
1134a24 
Use wp_trigger_error().
@anton-vlasenko
Update the docblock.
e40ce38
@anton-vlasenko
Update the docblock.
e582409 
Use wp_trigger_error().
@anton-vlasenko
Update the docblock.
cf0fc29 
Use wp_trigger_error().
@anton-vlasenko
Fix @return value.
acaf7dc 
Fix typo in the deprecated message.
@anton-vlasenko
Commit WIP.
17e4dc4
@anton-vlasenko
Implement unit test for WP_Term->data and magic methods.
b571215
@anton-vlasenko
Don't return the "data" class property in WP_Term::to_array().
d87a4f5
@anton-vlasenko
1. Implement new test cases for WP_Term->data.
f2d11a6 
2. Update docblock.
@anton-vlasenko
Refactor the unit test.
ab1885a
@anton-vlasenko
Add test case for the data property.
1ee82a9
@anton-vlasenko
Fix cs.
941abd0
@anton-vlasenko
Add the object_id property.
be9c8f5
@anton-vlasenko
Update the test.
9da3c3c
@anton-vlasenko
Declaring the $data property as private since the WP_Term class is de…
f42b14c 
…clared as final.
@anton-vlasenko
Declare public properties first.
50c176a
@anton-vlasenko
Add missing class properties.
c9075af
@anton-vlasenko
Add missing class properties (draft).
e110e59
@anton-vlasenko
Don't unset the filter property.
36467aa
@anton-vlasenko
Don't access property if it's not set.
67af5e0
@anton-vlasenko
Fix the tests.
6d3a183
@anton-vlasenko
Try to get rid of the $data property.
ddce8f4
@anton-vlasenko
Remove the data property and make it dynamic.
c39d689
@anton-vlasenko
Better PHPDoc blocks.
943f5ae
@anton-vlasenko anton-vlasenko force-pushed the fix/dynamic-properties-in-wp-term branch from 82daf2f to 7413002 Compare April 26, 2024 15:28
@anton-vlasenko anton-vlasenko changed the title Ensure WP_Term compatibility with PHP 8.2 and beyond May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@anton-vlasenko @rockitround