The current api doc for hook_schema() does not mention at all that the database tables returned by implementations will be automatically created. Yes, everyone KNOWS that. Except those who don't. Strangely, the doc for hook_install() actually provides this information, but it really should be in the hook_schema doc as well.

(Will roll a patch.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sven.lauer’s picture

Assigned: sven.lauer » Unassigned
Status: Active » Needs review
Issue tags: +Needs backport to D7
FileSize
1.32 KB

And a patch.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, this looks like a good addition to the docs! I also verified by looking at module_enable() and drupal_uninstall_modules() that the doc is correct about when things happen.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, doc-hook_schema-autoloaded-1312844-1.patch, failed testing.

sven.lauer’s picture

Status: Needs work » Needs review
FileSize
1.34 KB

Ah, I guess this patch conflicted with the one in #1307062: install file hooks should state whether module code is available. Re-rolling.

(If the testbot is happy, I'll set this back to RTBC, as nothing has changed.)

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Version: 8.x-dev » 7.x-dev

Thanks! Committed to 8.x. Leaving RTBC for 7.

catch’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

David_Rothstein’s picture

Status: Fixed » Needs work
+ * creation and alteration of the supported database engines *
  * See the Schema API Handbook at http://drupal.org/node/146843 for

Did someone type backspace when they meant to type a period?... Strange, they aren't even close to each other on the keyboard :)

catch’s picture

Version: 7.x-dev » 8.x-dev

Whoops.

jhodgdon’s picture

Issue tags: +Novice

Whoops indeed. At least three of us missed that. Thanks David!

Good novice issue to fix that problem...

kathyh’s picture

Status: Needs work » Needs review
FileSize
712 bytes

Since patch from comment#5 was already committed, this patch corrects only comment #10.

David_Rothstein’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks :)

jhodgdon’s picture

After committing, this needs to be D7 / to be ported, and a patch made combining #13 and #5. Thanks!

David_Rothstein’s picture

I think #5 was already committed to D7, actually? (In which case this followup can just go into both D7 and D8 at the same time.)

jhodgdon’s picture

True. :)

catch’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, doc-hook_schema-autoloaded-1312844-13.patch, failed testing.

kathyh’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Novice, +Needs backport to D7

The last submitted patch, doc-hook_schema-autoloaded-1312844-13.patch, failed testing.

kathyh’s picture

Status: Needs work » Needs review
FileSize
732 bytes

Re-rolled for D8 core dir change - 11/1/11

sven.lauer’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks for fixing (and re-rolling the fix for) my oversight!

This will need a re-re-roll for D7 once committed.

catch’s picture

Version: 8.x-dev » 7.x-dev

Committed/pushed to 8.x, thanks! Moving to 7.x.

sven.lauer’s picture

jhodgdon’s picture

Issue tags: +Quick fix

Thanks, looks like #13 is good for D7. Not very controversial to apply it. :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

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