Select MySQL as default database driver, and fix a E_ALL warning during install [#299330]

Comment	File	Size	Author
#7	panda.patch	1.97 KB	Anonymous (not verified)

#1	299330-sad-panda.patch	2.29 KB	Damien Tournoud

Comment #1

Damien Tournoud CreditAttribution: Damien Tournoud commented 24 August 2008 at 14:47

Title:	Select MySQL as default database driver	» Select MySQL as default database driver, and fix a E_ALL warning during install
Status:	Active	» Needs review

File	Size
299330-sad-panda.patch	2.29 KB

No panda shall remain sad.

Log in or register to post comments

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 24 August 2008 at 16:02

Ha, thanks!

Works fine. I asked why this is longer than a one-line change, and Damien pointed out that not all installations may have MySQL, so we should only default to it if it exists. Touché.

current(array_keys($drivers)) would be a bit more clear as just key($drivers).

+ $database = isset($databases['default']['default']) ? $databases['default']['default'] : array(); is weird. $database['default']['default']? I realize this isn't specific to this patch, but why a nested array like that? Can we add some documentation to that line?

Log in or register to post comments

Comment #3

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 24 August 2008 at 16:02

Status:

Needs review

» Needs work

Log in or register to post comments

Comment #4

Damien Tournoud CreditAttribution: Damien Tournoud commented 24 August 2008 at 16:08

current(array_keys($drivers)) would be a bit more clear as just key($drivers).

Not really. key($drivers) is the key of the current position in $drivers. current(array_keys($drivers)) is the first key of $drivers. There is no real guarantee that both will be equal.

+ $database = isset($databases['default']['default']) ? $databases['default']['default'] : array(); is weird. $database['default']['default']? I realize this isn't specific to this patch, but why a nested array like that? Can we add some documentation to that line?

The syntax of the $databases array is described in settings.php:

* For each database, you may optionally specify multiple "target" databases.
* A target database allows Drupal to try to send certain queries to a
* different database if it can but fall back to the default connection if not.
* That is useful for master/slave replication, as Drupal may try to connect
* to a slave server when appropriate and if one is not available will simply
* fall back to the single master server.
*
* The general format for the $databases array is as follows:
*
* $databases['default']['default'] = $info_array;
* $databases['default']['slave'][] = $info_array;
* $databases['default']['slave'][] = $info_array;

Log in or register to post comments

Comment #5

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 24 August 2008 at 16:38

Status:

Needs work

» Reviewed & tested by the community

Fair enough then. :)

Log in or register to post comments

Comment #6

Dries CreditAttribution: Dries commented 25 August 2008 at 07:53

Status:

Reviewed & tested by the community

» Needs work

Unfortunately, this panda will have to wait for a re-roll. Patch no longer applies. The end() trick might be better than the current trick.

Log in or register to post comments

Comment #7

Anonymous (not verified) CreditAttribution: Anonymous commented 25 August 2008 at 13:40

File	Size
panda.patch	1.97 KB

i created a duplicate issue for this.

i've attached an updated patch, but it still doesn't quite match the doco. do we want to handle the case i described in the issue above (which this patch doesn't manage), or do we need to update the docs?