require/import vs require/include in PHP Coding Standards Documentation. - Fix for #143 #144
Conversation
Fixed: require/import => require/include
Per @jrfnl's suggestion, made clear that autoloading only applies to classes.
Fixed missing space.
wordpress-coding-standards/php.md
Outdated
|
|
||
| [info] | ||
| Note that, unless you have implemented [autoloading](https://www.php.net/manual/en/language.oop5.autoload.php), the `use` statement won't automatically load whatever is being imported. You'll either need to set up autoloading or load the file containing the class/function/constant using a `require/import` statement, for the imported constructs to be loaded when used. | ||
| Note that, unless you have implemented [autoloading](https://www.php.net/manual/en/language.oop5.autoload.php), the `use` statement won't automatically load referenced classes. You'll either need to set up autoloading or load the file containing the class using a `require/include` statement, for the imported classes to be loaded when used. Autoloading is only applicable to classes; for functions and constants, you must always use `require` or `include`. |
There was a problem hiding this comment.
| Note that, unless you have implemented [autoloading](https://www.php.net/manual/en/language.oop5.autoload.php), the `use` statement won't automatically load referenced classes. You'll either need to set up autoloading or load the file containing the class using a `require/include` statement, for the imported classes to be loaded when used. Autoloading is only applicable to classes; for functions and constants, you must always use `require` or `include`. | |
| Note that, unless you have implemented [autoloading](https://www.php.net/manual/en/language.oop5.autoload.php), the `use` statement won't automatically load referenced classes. You'll either need to set up autoloading or load the file containing the class using a `require/include[_once]` statement. Autoloading is only applicable to classes; for functions and constants, you must always use `require[_once]` or `include[_once]`. |
There was a problem hiding this comment.
I definitely like the added [_once] in the new sentence. I'll already apply that second part.
I am personally unsure about the require/include[_once].
I personally read it as "'require/includestatement", and due to that "statement" I included the optional_once` for both.
If you want to make it clear, I personally would suggest:
[...] or load the file containing the class using require[_once] or include[_once]. [...]
Imho that improves readability. Otherwise I personally would keep require/include.
Your thoughts?
There was a problem hiding this comment.
@jrfnl I've added the before mentioned change. And changed order to alphabetical (first include then require) this matches the paragraph on "Writing include/require statements" in lines 81-83 of the same file.
Let me know your thoughts.
There was a problem hiding this comment.
The order was intentional - require should generally be preferred over include, which is why it was mentioned first.
There was a problem hiding this comment.
@jrfnl I agree with that point. What do you think about adjusting the other appearance of include/require to require/include, to match the same idea?
There was a problem hiding this comment.
I'd need to look at the complete doc to be sure (on the road now, so can't look), but in principle, I'd be open to such a change.
There was a problem hiding this comment.
@jrfnl I've updated the order, and I've added a subsentence to the documentation in the upper paragraph. My experience showed that include statements can result in silent malfunctions, which are a pain to track down. If you disagree adding this, I'm absolutely fine to omit that.
Applied suggestion from code review by @jrfnl
Further code review feedback. Changed order to alphabetikal order: include, require. To be consistent with Lines 81-83 (Writing include/require statements) and
Switched from alphabetical order, to recommendation order.
|
@jrfnl I've missed your reply and just taken care of your comments. |
Co-authored-by: Juliette <[email protected]>
|
Resolved @jrfnl |
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
Thanks @apermo ! This now looks good to go 👍🏻
@dingo-d @GaryJones Want to do a final review & do the honours ? (squash-merge please!)
3 participants
This fixes #143
Also taken care of @jrfnl 's comment on slack that autoloading only applies to classes and not to functions or constants.