Skip to content

Conversation

@t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Oct 22, 2025

Related core PR:

What?

We've committed the Note feature to core. I'll incorporate any PHP code changes made during that feedback process into Gutenberg.

How?

  • As for the REST API itself, redundant logic has been simplified.
  • As for unit tests, I followed the core implementation and changed class names, etc. This makes it easier to backport new unit tests into the core.

Testing Instructions

  • There are no changes to the behavior of the Notes feature.
  • All unit tests should be ✅.

@t-hamano t-hamano self-assigned this Oct 22, 2025
@t-hamano t-hamano added Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting labels Oct 22, 2025
@t-hamano t-hamano force-pushed the backport-core-comment-rest-api branch from e6634c1 to 7323056 Compare October 22, 2025 02:49
@t-hamano t-hamano added the No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core label Oct 22, 2025
@@ -1 +1 @@
<?php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, this file should be placed in lib/, not lib/compat/wordpres-6.9. The Notes feature will continue to be developed on Gutenberg, and the REST API itself is likely to be updated.

If a directory change is required, I will address it in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's a lib/compat/wordpres-6.9 for similar controller overrides. But you can check how other controllers are handled.

The only exception is theme.json and global styles, since Gutenberg needs to serve the latest version of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we need to extend the comments REST API for the 7.0 release, how would we go about doing that? Perhaps by creating another class in the lib/compat/wordpres-7.0 directory and overriding the entire method there? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think we usually just change directories then, or Controller_WP_7_0 extends Controller_WP_6_9 and only the latter is initialized.

The files for lib/ are rarely removed, while the compat files are cleaned up after we bump the minimum required version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or Controller_WP_7_0 extends Controller_WP_6_9 and only the latter is initialized.

I prefer this approach. Updated in dd0ce3d

@github-actions
Copy link

github-actions bot commented Oct 22, 2025

Flaky tests detected in 84eb106.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18716946979
📝 Reported issues:

@t-hamano t-hamano marked this pull request as ready for review October 22, 2025 10:31
@github-actions
Copy link

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @t-hamano!

@t-hamano t-hamano merged commit ce1f2bb into trunk Oct 22, 2025
34 checks passed
@t-hamano t-hamano deleted the backport-core-comment-rest-api branch October 22, 2025 13:38
@github-actions github-actions bot added this to the Gutenberg 22.0 milestone Oct 22, 2025
Mamaduka added a commit that referenced this pull request Oct 22, 2025
@cbravobernal cbravobernal added the [Type] Code Quality Issues or PRs that relate to code quality label Oct 29, 2025
@github-actions
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin, New Block, [Type] Iteration.
  • Labels found: Gutenberg Plugin, [Type] Code Quality, No Core Sync Required, [Feature] Notes.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants