As explained in #140860, for consistency we need all tables to be in singular.

Here is a patch that renames poll.module's tables {poll_choices} to {poll_choice} and {poll_votes} to {poll_vote}, profile.module's {profile_fields} to {profile_field} and {profile_values} to {profile_value}, and taxonomy.module's {vocabulary_node_types} table to {vocabulary_node_type}.

CommentFileSizeAuthor
drupal_singular_poll_profile_taxonomy_tables.patch33.13 KBrecidive

Comments

swentel’s picture

swentel commented

This got committed allready, cf http://drupal.org/cvs?commit=157053 but I'll let Dries set it on fixed :)

Status: Needs review » Needs work

The last submitted patch failed testing.

recidive’s picture

recidive commented
Status: Needs work » Fixed

Ok. Marking this fixed.

dww’s picture

dww
we/he/they
commented
Status: Fixed » Active

Our coding standards say:

Use plural or collective nouns for table names since they are sets and not scalar values.

If we were going to standardize, we should standardize on our own stated coding standard. :(

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Priority: Normal » Critical

This should not have been committed. It was not RTBC. Not sure if the testing bot tested it after it was committed from core, but this needs review. And this needs to be uncommitted ASAP.

keith.smith’s picture

keith.smith commented
Priority: Critical » Normal

Of course, the rest of that item on the standards page says:

(Others prefer table names to match field names and prefer singular table names. Drupal uses both: blocks, filters, users; but book, poll, role.)

Which, essentially, says "use plural unless you'd rather use singular". Whichever way this issue goes, we should update the coding standards appropriately.

keith.smith’s picture

keith.smith commented
Priority: Normal » Critical

Crossposted and accidentally changed priority.

recidive’s picture

recidive commented

Dries commited the patch when it was not breaking any tests and forgot to update the issue. Test bot ran again, and obviously failed to apply the patch.

After that #324313: Load multiple nodes and terms at once got re-rolled to match the table names changes and Dries commited that one too.

I suggest changing coding SQL coding conventions from:

"Use plural or collective nouns for table names since they are sets and not scalar values. (Others prefer table names to match field names and prefer singular table names. Drupal uses both: blocks, filters, users; but book, poll, role.)"

to:

"Use singular nouns for table names since they describe the entity the table represents."

Or something like that.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Active » Fixed

Seems to me clearly like new standards where agreed (by Dries and others) and as recidive suggests a simple update of the docs would make things consistently complete. After seeking for the standards, here hey are:

http://drupal.org/node/2497
I've updated the convention as suggested.

Status: Fixed » Closed (fixed)

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