- Issue created by @nicxvan
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
In π Split File hooks into separate classes Active I split out hooks based on common dependencies, and kept those with no dependencies in the
FileHooks
class. - π·πΊRussia Chi
Certain hook groups should be in a class together.
I would force using single class hooks wherever it is possible. There very few use cases when hooks are highly related to each other.
- π·πΊRussia Chi
Cross posted from π OOP hooks using event dispatcher Needs review . The "implements" seems redundant now.
/** * Implements hook_help(). */ #[Hook('help')] Do you think that "implements" note is redundant for this kind of hooks?
It gets worse in single hook classes.
/** * Implements hook_help(). */ #[Hook('help')] final class Help { /** * Implements hook_help(). */ public function __invoke(): void { } }
- πΊπΈUnited States nicxvan
Thank you for commenting.
Re 6 I think there are a bunch, especially if you have a shared helper method.
For 7, I agree, but what would you suggest for comments then? A description of what it is doing?
- π·πΊRussia Chi
@nicxvan we probably need to understand why "Implements hook_*" comment was initially added to Drupal coding standars. I guess it is needed to distinguish ordinary PHP functions from functions that implement Drupal hooks. Obviously with the
Hook
attribute is no longer needed. Also api.drupal.org might use this notation when listing hook implementatations.For me it seems that in most cases docblock in hooks is not needed at all.
This issue is very similar to #3376518: Allow omitting @var, @param and @return tags or their descriptions if the variable name and type is enough β . Maybe we should take same approach once it is resolved.
- πΊπΈUnited States nicxvan
It's so the api project can find them and document implementations, there is an issue to replace that.