There's a bit of discrepancy between what aspects of database API are key only and then inconsistent as to which order key and target are in the argument list. With 'default' being a common value for key and target this is going to get confused at some point.
from: grep key ./core/lib/Drupal/Core/Database/Database.php | grep function
Key only:
final public static function startLog($logging_key, $key = 'default') {
final public static function getLog($logging_key, $key = 'default') {
final public static function setActiveConnection($key = 'default') {
final public static function renameConnection($old_key, $new_key) {
final public static function removeConnection($key) {
Key followed by target.
public static function addConnectionInfo($key, $target, $info) {
final public static function getConnectionInfo($key = 'default') {
final protected static function openConnection($key, $target) {
public static function ignoreTarget($key, $target) {
Target followed by key.
final public static function getConnection($target = 'default', $key = NULL) {
public static function closeConnection($target = NULL, $key = NULL) {
Comment | File | Size | Author |
---|---|---|---|
#8 | issue-2225199.patch | 18.47 KB | danblack |
Comments
Comment #1
danblack CreditAttribution: danblack commented$key, $target is the right way to do it as this is how its referenced in settings.php. Though its bad to switch the API a bit of consistency is called for.
Comment #3
marcingy CreditAttribution: marcingy commentedThis isn't major nothing is broken
Comment #4
danblack CreditAttribution: danblack commented> This isn't major nothing is broken
quite right. Forgot to change this when I switch from Feature to Bug but even so major was excessive.
Comment #6
danblack CreditAttribution: danblack commentedComment #8
danblack CreditAttribution: danblack commentedComment #9
danblack CreditAttribution: danblack commentedComment #10
danblack CreditAttribution: danblack commentedDo the core/includes/database.inc db_* functions need to have key and target?
Comment #11
jhedstromI think this would have to wait until 8.1, or perhaps even 9.0?
Comment #12
danblack CreditAttribution: danblack commented> I think this would have to wait until 8.1, or perhaps even 9.0?
Don't you think that will make things just much worse? If you fix it early before people start using it these inconsistent APIs would be my approach. By hey, given the slow attention to bugs I've given up.
Comment #15
daffie CreditAttribution: daffie commentedA major +100 from me! It is very confusing. This is an API change and they are only allowed in a new major version.
Comment #16
catchIf 9.x only drops backwards compatibility with 8.x, then we need to consider how to do this in a bc-compatible way.
For example a new class with the new methods and deprecating the old one.