Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeytown2’s picture

Status: Active » Needs review
FileSize
1.68 KB

forgot to rename the function

Its the only hook missing
http://api.drupal.org/api/search/7/hook_taxonomy_

bleen’s picture

Seems really straightforward ... I just took a quick test drive and everything seems in order.

One more review for good luck and we can probably mark this RTBC

marcingy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

webchick’s picture

Priority: Critical » Normal
Issue tags: +Needs documentation, +API change

I want to be really careful here with making API changes this late in the game (even if they are "harmless" additions), because it sends the totally wrong signal about API freeze, and we need to be focused on releasing Drupal 7, not adding more shiny.

On the other hand, looking at http://api.drupal.org/api/search/7/hook_taxonomy_ it definitely seems clear that this is a simple API omission, rather than a "new" feature. Additionally, since terms are entities, the lack of a presave hook for terms was in fact a bug with the conformance to the entity interface, and since vocabularies are also entities, this qualifies here as well.

Committed to HEAD. But please watch it with that "critical" status and only use it for issues that are actually critical and cause Drupal to cease to function.

Also, since this is an API change, it needs to be documented on the Module update page.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
mikeytown2’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Fixed

I think that's fine.

Status: Fixed » Closed (fixed)
Issue tags: -Needs documentation, -API change

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