- Issue created by @nicxvan
- 🇨🇭Switzerland berdir Switzerland
Looks good to me. The plugin hook is a bit tricky to categorize as it's a generic hook, could argue that it's either a block or a layout builder hook. But no big deal, just a test module too. Not sure how much we even need to split hooks in test modules, but this one has quite a few.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇳🇿New Zealand danielveza Brisbane, AU
It's best not to RTBC issues that you've worked on and instead have another person review and mark it as RTBC.
In this case I've reviewed and I'm happy for this to be RTBC.
The only thoughts I had around this one is that it could be worth updating the title and/or description to make to clear that the existing hooks have also been split out into dedicated files. When I first reviewed the MR I was confused at the changes, since I only expected to see the functions in the .module file moved into hooks
- 🇺🇸United States nicxvan
Not sure why I did that with these two issues, maybe because they were test modules, I don't usually self RTBC like that.
The reorganization is in the IS, but I am happy to make that clearer.
I only did that here because it's a test module. - 🇳🇿New Zealand danielveza Brisbane, AU
No problem! Just thought I'd flag it so it didn't get blocked later when a committed reviewed this :)
Changes look good! And agree that since the work is already done, tests are passing and it's a test module that it's ok to do it all in one issue.