API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Plugin%21...

> Name of the alter hook.

This could do with a little more precision. To get a hook 'hook_sandwich_info_alter', you want to pass in only 'sandwich_info'.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Clarified the parameter $alter_hook by adding an example definition

JayeshSolanki’s picture

Status: Active » Needs review
Berdir’s picture

This is exactly the same behavior as drupal_alter()/ModuleHandlerInterface::alter() always has been, so maybe we should just reference to that?

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patch!

I would agree with #3, but this documentation is very short, so I think rather than *just* referencing it, we should go ahead and provide the docs, and also provide an @see link to ModuleHandlerInterface::alter().

Also, regardin the current patch:
- "Example" should not be capitalized in "For Example".
- Please do not use "foobar". Use a more representative hook name, like hook_mymodule_mydata_alter().
- In place of "to use", you might use wording like "If your alter hook is called...".
- In place of "you want to pass only", you might use more concise wording that tells what to do, such as just "pass in".

longwave’s picture

Status: Needs work » Needs review
FileSize
675 bytes

"mymodule_data" is the example used in ModuleHandlerInterface, so let's borrow that here.

longwave’s picture

FileSize
820 bytes

Or should we rename the parameter to make it more obvious it isn't just the hook name, like ModuleHandlerInterface does?

jhodgdon’s picture

Status: Needs review » Needs work

The docs there look good. The first comma (before "for example") should be a ; though.

Changing the parameter would be OK, but it looks like $type is the same parameter name as ModuleHandlerInterface::alter($type)?

Hmmm... Can the $type here be an array?
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Extension!ModuleH...

If so, I think we should definitely make a link to ModuleHandlerInterface::alter()... it seems unlikely for plugin managers that they would want to use an array though?

Anonymous’s picture

Hi I added a patch taking care of your recommendations hook_mymodule_data_alter()

jhodgdon’s picture

Good but needs a space after the ; -- thanks! Also, neither "for" nor "example" should be capitalized, since they are in the middle of a sentence.

Anonymous’s picture

Anonymous’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, that should do it.

longwave’s picture

Status: Reviewed & tested by the community » Needs work

There is a typo in #10, "mymodula_data"

Anonymous’s picture

@longwave Thanks for pointing out

Anonymous’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Doh! Missed that. Looks good this time. :)

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 8.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.