Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Determine mapping from Drush procedural hooks to new HookManager #2116

Closed
greg-1-anderson opened this issue Apr 4, 2016 · 4 comments
Closed
Milestone

Comments

@greg-1-anderson
Copy link
Member

The existing Drush command dispatcher provides the following hooks:

  • hook_init
  • hook_pre_validate
  • hook_validate
  • pre_hook
  • hook
  • post_hook

Also, all of these have a _rollback hook that is called iff the command fails after the corresponding hook has executed.

AnnotationCommand\HookManager provides the following hooks:

  • validate (validates arguments)
  • alter (changes result returned from primary command hook)
  • status (look up status result code from result object)
  • extract (get information to display to user from result object)

Each of these also have "pre" and "post" varieties, to give more flexibility vis-a-vis hook ordering (and for consistency). Note that many validate and alter hooks may run, but the first status or extract hook that successfully returns a result will halt processing of further hooks of the same type.

The mapping from Drush hook to AnnotationCommand hook is as follows:

  • hook_init => (none - needed)
  • hook_pre_validate => pre-validate
  • hook_validate => validate
  • pre_hook => post-validate
  • hook => (none - deprecated)
  • post_hook => alter
  • *_rollback => (none - tbd)

Maybe we need something better than post-validate to replace the old "pre_" hook.

It was fairly unusual for anyone to write a hook for some other command's primary callback function. This really should never be done, because it is indeterminate whether the hook will run before or after the primary callback function. It is therefore always better to use a pre or post hook to disambiguate what order was intended for the hook function.

Rollbacks are also fairly uncommon. We should investigate whether these can be done using Robo collections, and only provide a replacement for these if they are really needed.

@weitzman
Copy link
Member

weitzman commented Apr 4, 2016 via email

@greg-1-anderson
Copy link
Member Author

I edited the description above and added a brief description of the purpose of each of the new hooks. The hooks need better documentation in the AnnotationCommands project as well.

@greg-1-anderson
Copy link
Member Author

I updated the AnnotationCommand project description to contain a description of the available hooks.

@weitzman
Copy link
Member

weitzman commented Oct 1, 2016

This was completed by #2366.

@weitzman weitzman closed this as completed Oct 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants