First patch fixing this: removing getActiveConnection and keeping only getConnection and making openConnection return the connection instead of magically storing it. This makes following db_select/db_insert etc etc much easier because now all pieces in getConnection is visible and there is only one connection function to read. Impact on HEAD? I suspect none at all.

CommentFileSizeAuthor
#4 getConnection.patch23.59 KBchx
#3 getConnection.patch21.48 KBchx
#1 getConnection.patch21.48 KBchx
getConnection.patch20.26 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
FileSize
getConnection.patch21.48 KB

Now, the more-often used $target parameter is the first one. This allows really nice parameter passing.

Status: Needs review » Needs work

The last submitted patch failed testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review
FileSize
getConnection.patch21.48 KB

Tests now pass.

chx’s picture

chx CreditAttribution: chx commented
FileSize
getConnection.patch23.59 KB

Tests would pass if I would upload the right patch.

chx’s picture

chx CreditAttribution: chx commented

And the next issue is #363802: Document class methods with class, factory and defining interfaces

Crell’s picture

Crell CreditAttribution: Crell commented

chx and I discussed this at length in IRC before he submitted it, and I'm OK with it.

chx, when this lands can you also update the docs at: http://drupal.org/node/310070

Crell’s picture

Crell CreditAttribution: Crell commented
Priority: Critical » Normal
Status: Needs review » Reviewed & tested by the community

Er, yeah, changing status would be good too.

Dries’s picture

Dries CreditAttribution: Dries commented

This looks like a good simplification indeed -- I remember scratching my head about this when I reviewed early version of the new database abstraction layer. Doing some review and testing now.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks chx!

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Fixed » Reviewed & tested by the community

Uhm, the KLM lounge at Amsterdam/Schiphol airport does not allow me to do CVS commits ...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work

KLM lounge FAIL. ;)

Committed to HEAD. Docs please. :)

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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