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

Rest API: rename navigation fallback classes from WP_ to Gutenberg_ #51959

Merged
merged 5 commits into from
Jun 27, 2023
Merged

Rest API: rename navigation fallback classes from WP_ to Gutenberg_ #51959

merged 5 commits into from
Jun 27, 2023

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jun 27, 2023 •
edited
Loading

What?

Renames /compat/wordpress-6.3/class-wp-rest-navigation-fallback-controller.php / WP_REST_Navigation_Fallback_Controller and associated classes/filenames to the class-gutenberg-* and Gutenberg_* conventions.

Why?

The WP_REST_Navigation_Fallback_Controller class has been committed to core and therefore results in a naming conflict and unit test failures.

By renaming to Gutenberg_* we can avoid naming conflicts while still supporting 6.3 functionality in the plugin for users that won't upgrade to WordPress 6.3.

How?

Renaming stuff.

Testing Instructions

The unit tests should pass.

@ramonjd
The WP_REST_Navigation_Fallback_Controller class has been committed…
87c4e6c 
… to core and therefore results in a naming conflict and unit test failures.

Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`.
But we can conditionally load the file instead.
@ramonjd ramonjd self-assigned this Jun 27, 2023
@ramonjd ramonjd added [Type] Task Issues or PRs that have been broken down into an individual action to take [Type] Bug An existing feature does not function as intended labels Jun 27, 2023
@ramonjd
Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Bl…
ee38a17 
…ock_Menu_Converter

Load WP_REST_Navigation_Fallback_Controller dependencies in load.php
@ramonjd ramonjd requested review from talldan and getdave June 27, 2023 08:23
@ramonjd
Renamed all 6.3 classes to have the Gutenberg_ prefix. This should av…
007f7fe 
…oid compat errors and hopefully some confusion later.
@ramonjd ramonjd changed the title The WP_REST_Navigation_Fallback_Controller class has been committed… Rest API: rename navigation fallback classes from WP_ to Gutenberg_ Jun 27, 2023
@ramonjd ramonjd added [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. and removed [Type] Task Issues or PRs that have been broken down into an individual action to take labels Jun 27, 2023
talldan
talldan approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@talldan talldan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @ramonjd.

Looks like this is the approach other gutenebrg REST classes take, and tests are passing, so looks good to me!

@ramonjd ramonjd enabled auto-merge (squash) June 27, 2023 09:03
@talldan talldan disabled auto-merge June 27, 2023 09:23
@talldan talldan merged commit 2d1d0fa into trunk Jun 27, 2023
@talldan talldan deleted the try/load-compat-6.3-classes-conditionally branch June 27, 2023 09:24
@github-actions github-actions bot added this to the Gutenberg 16.2 milestone Jun 27, 2023
@talldan
Copy link
Contributor

talldan commented Jun 27, 2023

I merged this with a failing performance test. That test checks out trunk and compares performance with this branch. When it checked out trunk it was re-encountering the issue that this PR fixes. I believe merging it should fix things.

@getdave
Copy link
Contributor

getdave commented Jun 27, 2023

Thanks for handling this. So for my reference:

  • When authoring class like these in Plugin always use class-gutenberg-* and Gutenberg_*.
  • When backporting change all those instances to class-wp-* and WP_*.
  • Put files in the directory for the release in which the features were backported to WP Core.

Question: is this documented? If not where could I contribute a docs update?

@ramonjd
Copy link
Member Author

ramonjd commented Jun 27, 2023

Hi @getdave

No worries. You've got it spot on.

This document covers most of this, though not as succinctly as you've put it: https://github.com/WordPress/gutenberg/blob/trunk/lib/README.md

Probably needs updating to include classes etc

@getdave
Copy link
Contributor

getdave commented Jun 27, 2023

This seems to say the opposite of what I said above.

Perhaps it should say use Gutenberg until the point at when it's merged to Core?

@ramonjd
Copy link
Member Author

ramonjd commented Jun 27, 2023

Oops, that document says:

For features that may be merged to Core, it's best to use a wp_ prefix for functions or a WP_ prefix for classes.

So you were just following orders :)

This probably needs to be cleared up. During release I don't think it's practical to juggle consistencies between plugin and Core where both have wp_, and renaming during backport isn't that hard.

🤷

Sorry for the confusion. Despite that doc I think experience has led us to the class-gutenberg-* road.

I'll add a note to raise it with @noisysocks

@ramonjd ramonjd added the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Jun 28, 2023
@ramonjd ramonjd modified the milestones: Gutenberg 16.2, 16.1 Jun 28, 2023
@ramonjd
Copy link
Member Author

ramonjd commented Jun 28, 2023

I just cherry-picked this PR to the release/16.1 branch to get it included in the next release: 5a32952

ramonjd added a commit that referenced this pull request Jun 28, 2023
@ramonjd
Rest API: rename navigation fallback classes from WP_ to Gutenberg_ (#…
5a32952 
…51959)

* The `WP_REST_Navigation_Fallback_Controller` class has been committed to core and therefore results in a naming conflict and unit test failures.
Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`.
But we can conditionally load the file instead.

* Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Block_Menu_Converter
Load WP_REST_Navigation_Fallback_Controller dependencies in load.php

* Renamed all 6.3 classes to have the Gutenberg_ prefix. This should avoid compat errors and hopefully some confusion later.

* Also rename test files for completeness

* Updated deprecation notices to refer to Gutenberg classes
@ramonjd ramonjd removed the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Jun 28, 2023
ramonjd added a commit that referenced this pull request Jun 28, 2023
@ramonjd
Rest API: rename navigation fallback classes from WP_ to Gutenberg_ (#…
51c953b 
…51959)

* The `WP_REST_Navigation_Fallback_Controller` class has been committed to core and therefore results in a naming conflict and unit test failures.
Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`.
But we can conditionally load the file instead.

* Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Block_Menu_Converter
Load WP_REST_Navigation_Fallback_Controller dependencies in load.php

* Renamed all 6.3 classes to have the Gutenberg_ prefix. This should avoid compat errors and hopefully some confusion later.

* Also rename test files for completeness

* Updated deprecation notices to refer to Gutenberg classes
tellthemachines added a commit that referenced this pull request Jun 28, 2023
@tellthemachines @getdave
@ntsekouras @ramonjd @jameskoster @alexstine @t-hamano @noisysocks
Cherry-pick pre-6.3 Beta 1 bugfixes (#52011)
7366294 
* Restore sidebar in focus mode on Pattern click through in Browse Mode Library (#51897)

Brings back #51492

* [Command center]: Add preferences and keyboard shortcuts commands (#51862)

* [Command center]: Add preferences and keyboard shortcuts commands

* update labels

* [Site Editor]: Fix `library` command path (#51837)

* Updating social link attributes (#51997)

* Try: Update template titles (#51428)

* Update template titles

* Fix typo

Co-authored-by: Alex Stine <alex.stine@yourtechadvisors.com>

* Revert Index rename

* "front page" -> "homepage"

* Update 404

Page make more sense given the template appears in the Pages panel too.

* Front Page

* home title + description

* Revert Home name change, and move compat files

* separate code for wp versions

* update tests

---------

Co-authored-by: Alex Stine <alex.stine@yourtechadvisors.com>
Co-authored-by: ntsekouras <ntsekouras@outlook.com>

* Update text color (#51965)

* Modal: Add small top padding to the content so that avoid cutting off the visible outline when hovering items (#51829)

* Site Editor: Fix focus cutoff in add template modal

* Add padding-top to the modal content

* Remove unnecessary padding-top

* Remove unnecessary padding-top

* Update changelog

* Revert top padding from block patterns list

* Revert top padding from block patterns list

* Remove unnecessary changes

* Remove unnecessary changes

* Add CSS inline comment

* Change padding metrics

* Rest API: rename navigation fallback classes from WP_ to Gutenberg_ (#51959)

* The `WP_REST_Navigation_Fallback_Controller` class has been committed to core and therefore results in a naming conflict and unit test failures.
Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`.
But we can conditionally load the file instead.

* Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Block_Menu_Converter
Load WP_REST_Navigation_Fallback_Controller dependencies in load.php

* Renamed all 6.3 classes to have the Gutenberg_ prefix. This should avoid compat errors and hopefully some confusion later.

* Also rename test files for completeness

* Updated deprecation notices to refer to Gutenberg classes

* Fix phpunit failures (#51950)

* Fix phpunit failures

* Add comment

* Update comment with actual reason this fix works

---------

Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Co-authored-by: James Koster <james@jameskoster.co.uk>
Co-authored-by: Alex Stine <alex.stine@yourtechadvisors.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Robert Anderson <robert@noisysocks.com>
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
@ramonjd @sethrubenstein
Rest API: rename navigation fallback classes from WP_ to Gutenberg_ (W…
580e3e1 
…ordPress#51959)

* The `WP_REST_Navigation_Fallback_Controller` class has been committed to core and therefore results in a naming conflict and unit test failures.
Ideally `WP_REST_Navigation_Fallback_Controller` should have been named `WP_REST_Navigation_Fallback_Controller_Gutenberg` and extended `WP_REST_Navigation_Fallback_Controller`.
But we can conditionally load the file instead.

* Renamed WP_Classic_To_Block_Menu_Converter to Gutenberg_Classic_To_Block_Menu_Converter
Load WP_REST_Navigation_Fallback_Controller dependencies in load.php

* Renamed all 6.3 classes to have the Gutenberg_ prefix. This should avoid compat errors and hopefully some confusion later.

* Also rename test files for completeness

* Updated deprecation notices to refer to Gutenberg classes
@getdave getdave mentioned this pull request Aug 10, 2023
Open
@ramonjd ramonjd mentioned this pull request Aug 14, 2023
Merged
@SiobhyB SiobhyB added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@SiobhyB SiobhyB mentioned this pull request Sep 5, 2023
Closed
86 tasks
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@talldan talldan talldan approved these changes

@oandregal oandregal oandregal approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey spacedmonkey is a code owner

@getdave getdave Awaiting requested review from getdave

@TimothyBJacobs TimothyBJacobs Awaiting requested review from TimothyBJacobs TimothyBJacobs is a code owner

@tellthemachines tellthemachines Awaiting requested review from tellthemachines

Assignees

@ramonjd ramonjd

Labels
[Type] Bug An existing feature does not function as intended [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ramonjd @talldan @getdave @oandregal @SiobhyB