From 1fb58c8b5440ba7c43975fe3ea77a54698dbf764 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 14 Aug 2023 13:04:30 +1000 Subject: [PATCH 1/3] Updating lib/PHP advice with practical reality :) Adding stuff about syncing --- lib/README.md | 200 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 119 insertions(+), 81 deletions(-) diff --git a/lib/README.md b/lib/README.md index bbce167c72d94..0956afe89c26f 100644 --- a/lib/README.md +++ b/lib/README.md @@ -1,145 +1,183 @@ # Gutenberg PHP -## File structure +The Gutenberg plugin is continuously enhancing existing features and creating new ones. Some features, once considered stable and useful, are merged into Core (the WordPress source code) during a WordPress release. Others remain in the plugin forever or are eventually removed as the minimum supported WordPress version changes. -Gutenberg adds features to WordPress Core using PHP hooks and filters. Some -features, once considered stable and useful, are merged into Core during a Core -release. Some features remain in the plugin forever or are removed. +During a WordPress release, new features, bugfixes and other changes are "synced" between the Gutenberg plugin and WordPress Core. Consistent naming and directory structures make this process easier by preventing naming conflicts and compartmentalizing release-specific code. -To make it easier for contributors to know which features need to be merged to -Core and which features can be deleted, Gutenberg uses the following file -structure for its PHP code: +The following documentation is intended to act as a guide only. If you're unsure about naming or where to place new PHP files, please don't hesitate to ping other contributors on Github or ask in the #core-editor channel on [WordPress Slack](https://make.wordpress.org/chat/). -- `lib/experimental` - Experimental features that exist only in the plugin. They - are not ready to be merged to Core. -- `lib/stable` - Stable features that exist only in the plugin. They could one - day be merged to Core, but not yet. -- `lib/compat/wordpress-X.Y` - Stable features that are intended to be merged to - Core in the future X.Y release, or that were previously merged to Core in the - X.Y release and remain in the plugin for backwards compatibility when running - the plugin on older versions of WordPress. -- `lib/compat/plugin` - Features for backwards compatibility for the plugin consumers. These files don't need to be merged to Core and should have a timeline for when they should be removed from the plugin. +## File structure -## Best practices +To make it easier for contributors to identify features that should be merged into Core and those that can be deleted, Gutenberg uses the following file structure for its PHP code: -### Prefer the `wp` prefix +- `lib/experimental` - Experimental features that exist only in the plugin. They should not be merged into Core. +- `lib/compat/wordpress-X.Y` - Stable features that are intended to be merged into Core in a future `X.Y` release, or that were previously merged to Core in the `X.Y` release and remain in the plugin for backwards compatibility when running the plugin on older versions of WordPress. +- `lib/compat/plugin` - Features for backwards compatibility for the plugin consumers. These files don't need to be merged into Core and should have a timeline for when they should be removed from the plugin. -For features that may be merged to Core, it's best to use a `wp_` prefix for functions or a `WP_` prefix for classes. +Files at the root of `/lib` are generally considered to contain "evergreen" code. Such code is both fundamental to the proper functioning of the plugin, and also so often updated that versioning it between WordPress releases is not practical. Changes to these files are merged into Core as required. -This applies to both experimental and stable features. +## Best practices + +There are a few best practices that should be observed when adding new files to the Gutenberg plugin. -Using the `wp_` prefix avoids us having to rename functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. +### Using `gutenberg` suffixes/prefixes vs. `wp` prefixes -Functions that are intended solely for the plugin, e.g., plugin infrastructure, should use the `gutenberg_` prefix. +To avoid naming conflicts with WordPress Core and other plugins, the Gutenberg plugin uses the `gutenberg` identifier in many of its PHP classes and function names, e.g., `WP_Theme_JSON_Gutenberg` and `gutenberg_get_block_editor_settings`. -#### Feature that might be merged to Core +This is especially so for classes and functions whose functionality is ubiquitous and constantly being updated — so-called "evergreen" code. Anything related to `WP_Theme_JSON_Gutenberg` is a good example of this: this class controls the way Gutenberg processes and outputs global styles and much more, and its methods are called in many places. In every aspect of plugin functionality, we want plugin users to have access to the latest versions of these files, even if they are running an older version of WordPress. ```php -function wp_get_navigation( $slug ) { ... } +/** +* Returns something useful. +* +* @since 6.2.0 Updates to something even more useful. +* @since 6.3.0 Now more useful than ever. +* +* @return string Something useful. +*/ +function gutenberg_get_something_useful() { + // ... +} ``` -#### Plugin infrastructure that will never be merged to Core +When merging or updating these functions into Core, they must use the `wp_` prefix for functions or a `WP_` prefix for classes. ```php -function gutenberg_get_navigation( $slug ) { ... } +/** +* Returns something useful. +* +* @since 6.2.0 Updates to something even more useful. +* @since 6.3.0 Now more useful than ever. +* +* @return string Something useful. +*/ +function wp_get_something_useful() { + // ... +} ``` -### Group PHP code by _feature_ +Plugin code that is stable and expected to be merged "as-is" into Core in the near future can use the `wp_` prefix for functions or a `WP_` prefix for classes. -Developers should organize PHP into files or folders by _feature_, not by _component_. +Files can be placed within the `lib/compat/wordpress-X.Y` directory, where `X.Y` is the target version of WordPress. -When defining a function that will be hooked, developers should call `add_action` and `add_filter` immediately after the function declaration. - -These two practices make it easier for PHP code to start in one folder (e.g., `lib/experimental`) and eventually move to another using a simple `git mv`. +The caveat is that no functions or classes with the same names already exist in Core. A quick codebase search will also help you know if your new names are unique. -#### Good +Wrapping such code in `class_exists()` and `function_exists()` checks should be used to ensure it executes in the plugin up until it is merged to Core, or when running the plugin on older versions of WordPress. ```php -// lib/experimental/navigation.php +if ( ! function_exists( 'wp_a_new_and_stable_feature' ) ) { + /** + * A very new and stable feature. + * + * @return string Something useful. + */ + function wp_a_new_and_stable_feature() { + // ... + } +} +``` -function wp_get_navigation( $slug ) { ... } +Or for classes: -function wp_register_navigation_cpt() { ... } +```php +/** + * WP_A_Stable_Class class + * + * @package WordPress + * @since 6.3.0 + */ +if ( class_exists( 'WP_A_Stable_Class' ) ) { + return; +} -add_action( 'init', 'wp_register_navigation_cpt' ); +/** + * A very stable class that does something. + * + * @since 6.3.0 + */ +class WP_A_Stable_Class { ... } ``` -#### Not so good +Wrapping code in `class_exists()` and `function_exists()` is usually inappropriate for evergreen code, or any plugin code that we expect to undergo constant change, because it would prevent the latest versions of the code from being used. For example, the statement `class_exists( 'WP_Theme_JSON' )` would return `true` because the class already exists in Core. -```php -// lib/experimental/functions.php +When to use which prefix is a judgement call, but the general rule is that if you're unsure, use the `gutenberg` prefix because it will less likely to give rise to naming conflicts. What's more, it can be changed more easily after the fact. -function wp_get_navigation( $slug ) { ... } +As always, get in touch with your fellow contributors if you're unsure. -// lib/experimental/post-types.php +### Documentation and annotations -function wp_register_navigation_cpt() { ... } +For every class, method and function in the plugin, use the [WordPress PHP documentation standards](https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/) to document the code. -// lib/experimental/init.php -add_action( 'init', 'wp_register_navigation_cpt' ); -``` +It's particularly important to observe annotation standards, and `@since` descriptions that specify the target WordPress version, so that all contributors can easily identify what needs to be (or what already has been) merged to Core and when. -### Wrap functions and classes with `! function_exists` and `! class_exists` +Developers should also write a brief note about _how_ their feature should be merged to Core, for example, which Core file or function should be patched. -Developers should take care to not define functions and classes that are already defined. +Notes can be included in the doc comment. -When writing new functions and classes, it's good practice to use `! function_exists` and `! class_exists`. +This helps future developers know what to do when merging Gutenberg features into Core. -If Core has defined a symbol once and then Gutenberg defines it a second time, fatal errors will occur. +```php +/** + * Returns a navigation object for the given slug. + * + * Should live in `wp-includes/navigation.php` when merged to Core. + * + * @since 6.3.0 + * + * @param string $slug + * @return WP_Navigation + */ +function wp_get_navigation( $slug ) { ... } +``` + +### Group PHP code by _feature_ -Wrapping functions and classes avoids such errors if the feature is merged to Core. +Developers should organize PHP into files or folders by _feature_, not by _component_. + +When defining a function that will be hooked, developers should call `add_action` and `add_filter` immediately after the function declaration. + +These two practices make it easier for PHP code to start in one folder (e.g., `lib/experimental`) and eventually move to another using a simple `git mv`. #### Good ```php -// lib/experimental/navigation/navigation.php - -if ( ! function_exists( 'wp_get_navigation' ) ) { - function wp_get_navigation( $slug ) { ... } -} +// lib/experimental/navigation.php -// lib/experimental/navigation/class-wp-navigation.php +function wp_get_navigation( $slug ) { ... } -if ( class_exists( 'WP_Navigation' ) ) { - return; -} +function wp_register_navigation_cpt() { ... } -class WP_Navigation { ... } +add_action( 'init', 'wp_register_navigation_cpt' ); ``` #### Not so good ```php -// lib/experimental/navigation/navigation.php +// lib/experimental/functions.php function wp_get_navigation( $slug ) { ... } -// lib/experimental/navigation/class-gutenberg-navigation.php +// lib/experimental/post-types.php + +function wp_register_navigation_cpt() { ... } -class WP_Navigation { ... } +// lib/experimental/init.php +add_action( 'init', 'wp_register_navigation_cpt' ); ``` -Furthermore, a quick codebase search will also help you know if your new method is unique. +### Requiring files in lib/load.php +Should the load order allow it, try to group imports according to WordPress release, then feature. It'll help everyone to quickly recognise the files that belong to specific WordPress releases. -### Note how your feature should look when merged to Core +Existing comments in `lib/load.php` should act as a guide. -Developers should write a brief note about how their feature should be merged to Core, for example, which Core file or function should be patched. +## When to sync changes to Gutenberg PHP with Core and vice versa -Notes can be included in the doc comment. +If you've changed or added PHP files to the Gutenberg plugin, you'll need to confirm whether the changes are to be synced to WordPress Core, and therefore featured in the next release of WordPress. -This helps future developers know what to do when merging Gutenberg features into Core. +The Gutenberg Github pull request in question should be labeled with the `Needs PHP backport` label if the changes are to be synced to Core. -#### Good +If so, it is recommended to create a [new Trac ticket](https://core.trac.wordpress.org/newticket) and submit a pull request to the [WordPress Core Github repository](https://github.com/WordPress/wordpress-develop) soon after your pull request is merged. -```php -/** - * Returns a navigation object for the given slug. - * - * Should live in `wp-includes/navigation.php` when merged to Core. - * - * @param string $slug - * - * @return WP_Navigation - */ -function wp_get_navigation( $slug ) { ... } -``` +So too, if you've made changes in WordPress Core to code that also lives in the Gutenberg plugin, these changes will need to be synced (often called "backporting") to Gutenberg. The relevant Gutenberg Github pull request should be labeled with the `Backport from WordPress Core` label. + +If you're unsure, you can always ask for help in the #core-editor channel in [WordPress Slack](https://make.wordpress.org/chat/). From bddfc4b4a0952cb68f7552e87477b2e0643540fa Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 14 Aug 2023 14:10:08 +1000 Subject: [PATCH 2/3] Clarifying stuff --- lib/README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/README.md b/lib/README.md index 0956afe89c26f..fb4785ad69681 100644 --- a/lib/README.md +++ b/lib/README.md @@ -40,7 +40,7 @@ function gutenberg_get_something_useful() { } ``` -When merging or updating these functions into Core, they must use the `wp_` prefix for functions or a `WP_` prefix for classes. +When porting new functions into Core, the function must be renamed to use the `wp_` prefix for functions or a `WP_` prefix for classes. ```php /** @@ -58,8 +58,6 @@ function wp_get_something_useful() { Plugin code that is stable and expected to be merged "as-is" into Core in the near future can use the `wp_` prefix for functions or a `WP_` prefix for classes. -Files can be placed within the `lib/compat/wordpress-X.Y` directory, where `X.Y` is the target version of WordPress. - The caveat is that no functions or classes with the same names already exist in Core. A quick codebase search will also help you know if your new names are unique. Wrapping such code in `class_exists()` and `function_exists()` checks should be used to ensure it executes in the plugin up until it is merged to Core, or when running the plugin on older versions of WordPress. @@ -98,15 +96,15 @@ if ( class_exists( 'WP_A_Stable_Class' ) ) { class WP_A_Stable_Class { ... } ``` -Wrapping code in `class_exists()` and `function_exists()` is usually inappropriate for evergreen code, or any plugin code that we expect to undergo constant change, because it would prevent the latest versions of the code from being used. For example, the statement `class_exists( 'WP_Theme_JSON' )` would return `true` because the class already exists in Core. +Wrapping code in `class_exists()` and `function_exists()` is usually inappropriate for evergreen code, or any plugin code that we expect to undergo constant change between WordPress releases, because it would prevent the latest versions of the code from being used. For example, the statement `class_exists( 'WP_Theme_JSON' )` would return `true` because the class already exists in Core. -When to use which prefix is a judgement call, but the general rule is that if you're unsure, use the `gutenberg` prefix because it will less likely to give rise to naming conflicts. What's more, it can be changed more easily after the fact. +When to use which prefix is a judgement call, but the general rule is that if you're unsure, use the `gutenberg` prefix because it will less likely give rise to naming conflicts. As always, get in touch with your fellow contributors if you're unsure. ### Documentation and annotations -For every class, method and function in the plugin, use the [WordPress PHP documentation standards](https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/) to document the code. +For every class, method and function in the plugin, refer to the [WordPress PHP documentation standards](https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/) when documenting your code. It's particularly important to observe annotation standards, and `@since` descriptions that specify the target WordPress version, so that all contributors can easily identify what needs to be (or what already has been) merged to Core and when. From 9686656f34c4b46bb30e14b023c26d35127d1506 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 14 Aug 2023 16:35:25 +1000 Subject: [PATCH 3/3] Improving sentence about duplicate names. Props @andrewserong --- lib/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/README.md b/lib/README.md index fb4785ad69681..8239ae40fd8ef 100644 --- a/lib/README.md +++ b/lib/README.md @@ -58,7 +58,7 @@ function wp_get_something_useful() { Plugin code that is stable and expected to be merged "as-is" into Core in the near future can use the `wp_` prefix for functions or a `WP_` prefix for classes. -The caveat is that no functions or classes with the same names already exist in Core. A quick codebase search will also help you know if your new names are unique. +When doing so, care must be taken to ensure that no duplicate declarations to create functions or classes exist between Gutenberg and WordPress core code. A quick codebase search will also help you know if your new names are unique. Wrapping such code in `class_exists()` and `function_exists()` checks should be used to ensure it executes in the plugin up until it is merged to Core, or when running the plugin on older versions of WordPress.