- Issue created by @mstrelan
- Merge request !10278Issue #3488823: Add void return to entity_crud hook implementations β (Open) created by mstrelan
- π¦πΊAustralia mstrelan
Missed a few that don't have the right docs, i.e. "Implements hook_ENTITY_TYPE_":
- Drupal\block_content_test\Hook\BlockContentTestHooks::blockContentInsert()
- Drupal\block_content_test\Hook\BlockContentTestHooks::blockContentPresave()
- Drupal\block_content_test\Hook\BlockContentTestHooks::blockContentUpdate()
- Drupal\block_content_test\Hook\BlockContentTestHooks::blockContentView()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestDelete()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestInsert()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestLoad()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestPredelete()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestPresave()
- Drupal\config_test\Hook\ConfigTestHooksHooks::configTestUpdate()
- π¦πΊAustralia mstrelan
I think
hook_entity_bundle_create
is meant to be in the entity_crud group too, but the doc has @see instead of @ingroup, which we should fix too. That would give us the following extras:- Drupal\field_ui\Hook\FieldUiHooks::entityBundleCreate()
- Drupal\jsonapi\Hook\JsonapiHooks::entityBundleCreate()
- Drupal\entity_schema_test\Hook\EntitySchemaTestHooks::entityBundleCreate()
And some more for the list in #4:
- \Drupal\field_ui\Hook\FieldUiHooks::entityViewModePresave
- \Drupal\field_ui\Hook\FieldUiHooks::entityFormModePresave
- \Drupal\field_ui\Hook\FieldUiHooks::entityViewModeDelete
- \Drupal\field_ui\Hook\FieldUiHooks::entityFormModeDelete
- πΊπΈUnited States nicxvan
I'll finish reviewing later this is a lot.
In the meantime can you add any with incorrect implements comments here https://www.drupal.org/project/drupal/issues/3488948 π Confirm conversions where implements was incorrect Active
Across all of these hook issues? We need to confirm the hook still has the extra type.
- πΊπΈUnited States nicxvan
Ok I took a look through and I don't see any other exceptions either.
- π¦πΊAustralia mstrelan
A few more:
\Drupal\image_module_test\Hook\ImageModuleTestHooks::imageStylePresave \Drupal\layout_builder\Hook\LayoutBuilderHooks::fieldConfigDelete \Drupal\layout_builder\Hook\LayoutBuilderHooks::fieldConfigInsert \Drupal\menu_link_content\Hook\MenuLinkContentHooks::menuDelete
- πΊπΈUnited States nicxvan
\Drupal\layout_builder\Hook\LayoutBuilderHooks::fieldConfigDelete
\Drupal\layout_builder\Hook\LayoutBuilderHooks::fieldConfigInsertCan be combined:
Delete one Move the hook to the other along with the implements line to the other. Rename the method as well so it's not referencing one hook or the other!I also confirmed these ones, I'll move them to π Confirm conversions where implements was incorrect Active for completeness.
- π¦πΊAustralia mstrelan
Re #9: there are plenty of hooks that could be refactored one way or another, but that's not really in scope here, so I'd prefer to leave that out.
- πΊπΈUnited States smustgrave
Needs a rebase
If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
- Status changed to Needs review
about 24 hours ago 5:48am 21 January 2025