From 476964f4cf4c91d300af59cbee9dd90217f2d612 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Mon, 21 Mar 2022 17:59:07 +1100 Subject: [PATCH 1/8] Add lib/README.md --- lib/README.md | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 lib/README.md diff --git a/lib/README.md b/lib/README.md new file mode 100644 index 00000000000000..acfb84b0b0c52c --- /dev/null +++ b/lib/README.md @@ -0,0 +1,158 @@ +# Gutenberg PHP + +## File structure + +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. + +To make it easier for contribtors to know which features need to be merged to +Core and which features can be deleted, Gutenberg uses the following file +strucutre for its PHP code: + +- `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. + +## Best practices + +### Prefer the `wp` prefix + +For features that may be merged to Core, it's best to use a `wp_` prefix for +functions or a `WP_` prefix for classes. This applies to both experimental and +stable features. + +This meakes it easier for contributors and plugin developers as there is less +cumbersome renaming of functions and classes from `gutenberg_` to `wp_` if the +feature is merged to Core. + +Functions that are intended solely for the plugin, e.g. plugin infastrucutre, +should use the `gutenberg_` prefix. + +#### Bad + +```php +function gutenberg_get_navigation( $slug ) { +} +``` + +#### Good + +```php +function wp_get_navigation( $slug ) { +} +``` + +### Group PHP code by _feature_ + +Developers should organize their PHP into files or folders by _feature_, not by +_component_. + +Relatedly, developers should call `add_action` and `add_filter` immediately +after the function being hooked is defined. + +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`. + +#### Bad + +```php +// lib/experimental/functions.php + +function wp_get_navigation( $slug ) { +} + +// lib/experimental/post-types.php + +function wp_register_navigation_cpt() { +} + +// lib/experimental/init.php +add_action( 'init', 'wp_register_navigation_cpt' ); +``` + +#### Good + +```php +// lib/experimental/navigation.php + +function wp_get_navigation( $slug ) { +} + +function wp_register_navigation_cpt() { +} +add_action( 'init', 'wp_register_navigation_cpt' ); +``` + +### Wrap functions and classes with `! function_exists` and `! class_exists` + +Developers should take care to not define a function or class if it already +is defined. + +This ensures that, if the feature is merged to Core, that there are no fatal +errors caused by Core defining the symbol once and then Gutenberg defining it a +second time. + +#### Bad + +```php +// lib/experimental/navigation/navigation.php + +function wp_get_navigation( $slug ) { +} + +// lib/experimental/navigation/class-gutenberg-navigation.php + +class WP_Navigation { +} +``` + +#### Good + +```php +// lib/experimental/navigation/navigation.php + +if ( ! function_exists( 'wp_get_navigation' ) ) { + function wp_get_navigation( $slug ) { + } +} + +// lib/experimental/navigation/class-wp-navigation.php + +if ( class_exists( 'WP_Navigation' ) ) { + return; +} + +class WP_Navigation { +} +``` + +### Note how your feature should look when merged to Core + +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. This can be +included in the doc comment. + +This helps future developers know what to do when merging Gutenberg features +into Core. + +#### Good + +```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 ) { +} +``` From abbfb8b8f146efb4ffd1e8bd4b84d88837ac00c3 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 21 Mar 2022 19:59:46 +1100 Subject: [PATCH 2/8] Subediting --- lib/README.md | 92 +++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 51 deletions(-) diff --git a/lib/README.md b/lib/README.md index acfb84b0b0c52c..a891b95890f7ce 100644 --- a/lib/README.md +++ b/lib/README.md @@ -8,7 +8,7 @@ release. Some features remain in the plugin forever or are removed. To make it easier for contribtors to know which features need to be merged to Core and which features can be deleted, Gutenberg uses the following file -strucutre for its PHP code: +structure for its PHP code: - `lib/experimental` - Experimental features that exist only in the plugin. They are not ready to be merged to Core. @@ -27,25 +27,20 @@ For features that may be merged to Core, it's best to use a `wp_` prefix for functions or a `WP_` prefix for classes. This applies to both experimental and stable features. -This meakes it easier for contributors and plugin developers as there is less -cumbersome renaming of functions and classes from `gutenberg_` to `wp_` if the -feature is merged to Core. +This minimizes the cumbersome process of renaming functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. -Functions that are intended solely for the plugin, e.g. plugin infastrucutre, -should use the `gutenberg_` prefix. +Functions that are intended solely for the plugin, e.g. plugin infrastructure, should use the `gutenberg_` prefix. -#### Bad +#### Good ```php -function gutenberg_get_navigation( $slug ) { -} +function wp_get_navigation( $slug ) {} ``` -#### Good +#### Not so good ```php -function wp_get_navigation( $slug ) { -} +function gutenberg_get_navigation( $slug ) {} ``` ### Group PHP code by _feature_ @@ -53,64 +48,48 @@ function wp_get_navigation( $slug ) { Developers should organize their PHP into files or folders by _feature_, not by _component_. -Relatedly, developers should call `add_action` and `add_filter` immediately +Also, developers should call `add_action` and `add_filter` immediately after the function being hooked is defined. 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`. -#### Bad +#### Good ```php -// lib/experimental/functions.php +// lib/experimental/navigation.php -function wp_get_navigation( $slug ) { -} +function wp_get_navigation( $slug ) { ... } -// lib/experimental/post-types.php - -function wp_register_navigation_cpt() { -} +function wp_register_navigation_cpt() { ... } -// lib/experimental/init.php add_action( 'init', 'wp_register_navigation_cpt' ); ``` -#### Good +#### Not so goo ```php -// lib/experimental/navigation.php +// lib/experimental/functions.php -function wp_get_navigation( $slug ) { -} +function wp_get_navigation( $slug ) { ... } -function wp_register_navigation_cpt() { -} +// lib/experimental/post-types.php + +function wp_register_navigation_cpt() { ... } + +// lib/experimental/init.php add_action( 'init', 'wp_register_navigation_cpt' ); ``` ### Wrap functions and classes with `! function_exists` and `! class_exists` -Developers should take care to not define a function or class if it already -is defined. +Developers should take care to not define functions and classes that are already defined. -This ensures that, if the feature is merged to Core, that there are no fatal -errors caused by Core defining the symbol once and then Gutenberg defining it a -second time. +When writing new function and classes, it's good practice to use `! function_exists` and `! class_exists`. -#### Bad +If Core has defined a symbol once and then Gutenberg defines it a second time, fatal errors will occur. -```php -// lib/experimental/navigation/navigation.php - -function wp_get_navigation( $slug ) { -} - -// lib/experimental/navigation/class-gutenberg-navigation.php - -class WP_Navigation { -} -``` +Wrapping functions and classes avoids such errors if the feature is merged to Core. #### Good @@ -118,8 +97,7 @@ class WP_Navigation { // lib/experimental/navigation/navigation.php if ( ! function_exists( 'wp_get_navigation' ) ) { - function wp_get_navigation( $slug ) { - } + function wp_get_navigation( $slug ) { ... } } // lib/experimental/navigation/class-wp-navigation.php @@ -128,10 +106,23 @@ if ( class_exists( 'WP_Navigation' ) ) { return; } -class WP_Navigation { -} +class WP_Navigation { ... } ``` +#### Not so good + +```php +// lib/experimental/navigation/navigation.php + +function wp_get_navigation( $slug ) { ... } + +// lib/experimental/navigation/class-gutenberg-navigation.php + +class WP_Navigation { ... } +``` + +Furthermore, a quick codebase search will also help you know if your new method is unique. + ### Note how your feature should look when merged to Core Developers should write a brief note about how their feature should be merged to @@ -153,6 +144,5 @@ into Core. * * @return WP_Navigation */ -function wp_get_navigation( $slug ) { -} +function wp_get_navigation( $slug ) { ... } ``` From d43351b60a699edb2b7a0e70caa6e4e1eb05e436 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 21 Mar 2022 20:22:20 +1100 Subject: [PATCH 3/8] Formatting --- lib/README.md | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/README.md b/lib/README.md index a891b95890f7ce..136db393586644 100644 --- a/lib/README.md +++ b/lib/README.md @@ -23,36 +23,33 @@ structure for its PHP code: ### Prefer the `wp` prefix -For features that may be merged to Core, it's best to use a `wp_` prefix for -functions or a `WP_` prefix for classes. This applies to both experimental and -stable features. +For features that may be merged to Core, it's best to use a `wp_` prefix for functions or a `WP_` prefix for classes. -This minimizes the cumbersome process of renaming functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. +This applies to both experimental and stable features. -Functions that are intended solely for the plugin, e.g. plugin infrastructure, should use the `gutenberg_` prefix. +Using the `wp_` prefix facilitates the process of renaming functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. + +Functions that are intended solely for the plugin, e.g., plugin infrastructure, should use the `gutenberg_` prefix. #### Good ```php -function wp_get_navigation( $slug ) {} +function wp_get_navigation( $slug ) { ... } ``` #### Not so good ```php -function gutenberg_get_navigation( $slug ) {} +function gutenberg_get_navigation( $slug ) { ... } ``` ### Group PHP code by _feature_ -Developers should organize their PHP into files or folders by _feature_, not by -_component_. +Developers should organize PHP into files or folders by _feature_, not by _component_. -Also, developers should call `add_action` and `add_filter` immediately -after the function being hooked is defined. +Also, developers should call `add_action` and `add_filter` immediately after the function being hooked is defined. -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`. +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 @@ -66,7 +63,7 @@ function wp_register_navigation_cpt() { ... } add_action( 'init', 'wp_register_navigation_cpt' ); ``` -#### Not so goo +#### Not so good ```php // lib/experimental/functions.php @@ -125,12 +122,11 @@ Furthermore, a quick codebase search will also help you know if your new method ### Note how your feature should look when merged to Core -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. This can be -included in the doc comment. +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. + +Notes can be included in the doc comment. -This helps future developers know what to do when merging Gutenberg features -into Core. +This helps future developers know what to do when merging Gutenberg features into Core. #### Good From be12599bfd258baf8000f044501294bbce2cb14c Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Tue, 22 Mar 2022 14:15:26 +1100 Subject: [PATCH 4/8] Fix typo Co-authored-by: Daniel Richards --- lib/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/README.md b/lib/README.md index 136db393586644..f24e2e11d9282f 100644 --- a/lib/README.md +++ b/lib/README.md @@ -6,7 +6,7 @@ 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. -To make it easier for contribtors to know which features need to be merged to +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: From a32abde2d163cd6de2c1d5d54f53b7e028272448 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Tue, 22 Mar 2022 14:19:04 +1100 Subject: [PATCH 5/8] Clarify why we want the wp_ prefix --- lib/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/README.md b/lib/README.md index f24e2e11d9282f..7c10cd9e580840 100644 --- a/lib/README.md +++ b/lib/README.md @@ -27,7 +27,7 @@ For features that may be merged to Core, it's best to use a `wp_` prefix for fun This applies to both experimental and stable features. -Using the `wp_` prefix facilitates the process of renaming functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. +Using the `wp_` prefix avoids us having to rename functions and classes from `gutenberg_` to `wp_` if the feature is merged to Core. Functions that are intended solely for the plugin, e.g., plugin infrastructure, should use the `gutenberg_` prefix. From b3fe3f60c672ea3d6c7f9ed89950f8a10bc0ca12 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Tue, 22 Mar 2022 14:19:39 +1100 Subject: [PATCH 6/8] Reword sentence about hooking --- lib/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/README.md b/lib/README.md index 7c10cd9e580840..02730a6a02c9b1 100644 --- a/lib/README.md +++ b/lib/README.md @@ -47,7 +47,7 @@ function gutenberg_get_navigation( $slug ) { ... } Developers should organize PHP into files or folders by _feature_, not by _component_. -Also, developers should call `add_action` and `add_filter` immediately after the function being hooked is defined. +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`. From dcb2d7e00e165c86756ae6cc92e3ff3f6536bc99 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Tue, 22 Mar 2022 14:19:58 +1100 Subject: [PATCH 7/8] Fix typo --- lib/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/README.md b/lib/README.md index 02730a6a02c9b1..498b8a07cf8090 100644 --- a/lib/README.md +++ b/lib/README.md @@ -82,7 +82,7 @@ add_action( 'init', 'wp_register_navigation_cpt' ); Developers should take care to not define functions and classes that are already defined. -When writing new function and classes, it's good practice to use `! function_exists` and `! class_exists`. +When writing new functions and classes, it's good practice to use `! function_exists` and `! class_exists`. If Core has defined a symbol once and then Gutenberg defines it a second time, fatal errors will occur. From b2cdd80a9ffad46dfa7f982e948f2d01c917693a Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Tue, 22 Mar 2022 14:21:20 +1100 Subject: [PATCH 8/8] Clarify that gutenberg_ is acceptable for infrastrucutre --- lib/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/README.md b/lib/README.md index 498b8a07cf8090..5636d17b286a82 100644 --- a/lib/README.md +++ b/lib/README.md @@ -31,13 +31,13 @@ Using the `wp_` prefix avoids us having to rename functions and classes from `gu Functions that are intended solely for the plugin, e.g., plugin infrastructure, should use the `gutenberg_` prefix. -#### Good +#### Feature that might be merged to Core ```php function wp_get_navigation( $slug ) { ... } ``` -#### Not so good +#### Plugin infrastructure that will never be merged to Core ```php function gutenberg_get_navigation( $slug ) { ... }