Download & Extend

Improve table prefix field's description in installer

Project:Drupal core
Version:6.x-dev
Component:documentation
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Today when I was testing the new HEAD release, I of course wanted to throw as many silly things at it as possible, so I went to add a table prefix. Then I caught myself because I couldn't remember if the prefix should be "head_" or "head" ... patch upcoming.

Comments

#1

Status:active» needs review

Here we go.

Btw, I went with "cms_" rather than "drupal_" because we don't have "Drupal" mentioned anywhere in this text, by design.

AttachmentSizeStatusTest resultOperations
drupal-table-prefix-example-172396-1.patch922 bytesIgnored: Check issue status.NoneNone

#2

Two quick thoughts: 1) some people don't know what "cms" stands for or means 2) the rest of the string suggests that the prefix should differ by site. So I think a slightly better example would be "site_" or "domain_".

#3

Title:Provide an example of what a table prefix is» Improve table prefix field's description in installer
Status:needs review» needs work

Good points. cms_ is definitely a bad choice.

However, you've highlighted another bug with this string... the prefixing is not so much about how many websites are sharing a database, but how many *applications* are sharing a database (or, I guess "application instances," but that's a bit technical). I could have PHPBB and Drupal on the same website, and they are going to get very cranky if they're both looking for a 'users' table. So I've fixed that too.

"drupal_" is actually the string this ought to be, but can't because of the install profile problem. So here's a new patch that sets this based on the profile internal name, which will always be lower-case w/ underscores. I make an exception for 'default' because 'default_' doesn't make a lot of sense as a prefix, but 'civicspace_' and 'drupaled_' would.

AttachmentSizeStatusTest resultOperations
drupal-table-prefix-example-172396-3.patch1.18 KBIgnored: Check issue status.NoneNone

#4

Status:needs work» needs review

Oops. Didn't mean to change status.

#5

Status:needs review» reviewed & tested by the community

Still applies with offset. Changes look sensible to me.

#6

Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks.

#7

Status:fixed» closed (fixed)

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

nobody click here