Simplify provider-side hooks
Aron Novak - June 11, 2009 - 12:49
| Project: | OpenID Content Profile Field |
| Version: | 6.x-6.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Aron Novak |
| Status: | needs review |
Jump to:
Description
See http://groups.drupal.org/node/21357
hook_openid_provider_ax($op, $attributes, $request) and
hook_openid_provider_sreg($op, $sreg_fields, $request)
is too much.
Almost the same signature, almost the same purpose. Let's unify them.
Also in the patchset (depending patches): #488628: Simplify provider-side hooks , #488636: Simplify provider-side hooks
| Attachment | Size |
|---|---|
| openid_cp_field_union_provider_hooks.patch | 2.54 KB |
#1
Patchset also includes #489776: Simplify provider-side hooks at OpenID stack
#2
I'm actually not convinced whether this consolidation is a good idea, here are my reasons:
- the change has only minor benefits in making the code clearer in openid_cp_field, there may be other ways to achieve this.
- there is a certain clarity in linking the hook to a specific invoker module (i. e. sreg invokes an _sreg hook)
- it comes at the cost of patching 4 projects
- it moves against the trend in Drupal to break out $op style hooks into their own hooks
I may be overlooking some aspect here, is there any other issue this patch would be solving?
#3
There is no additional benefit on the top of decreasing the number of the hooks at openid stack and making the code a little bit simplier in openid_cp_field.
"it comes at the cost of patching 4 projects" - what are under heavy development
"it moves against the trend in Drupal to break out $op style hooks into their own hooks" - $op is not affected by the patch, anyway, $op is not used really, it can be eliminated.
"there is a certain clarity in linking the hook to a specific invoker module" - agree