Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Updated: Comment #N
Problem/Motivation
The DatabaseLockBackend class/service still calls db_* procedural functions.
Proposed resolution
Inject the database connection as a dependency and add the database service in the DIC.
Remaining tasks
Patch, etc...
User interface changes
None
API changes
None
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#12 | 2087931-12.patch | 5.21 KB | damiankloip |
#12 | interdiff-2087931-12.txt | 597 bytes | damiankloip |
#7 | 2087931-5.patch | 5.22 KB | damiankloip |
#7 | interdiff-2087931-5.txt | 3.5 KB | damiankloip |
#2 | 2087931-2.patch | 5.24 KB | damiankloip |
Comments
Comment #2
damiankloip CreditAttribution: damiankloip commentedComment #3
ParisLiakos CreditAttribution: ParisLiakos commentedyes!
Comment #4
webchickCan we please name this $database instead? The code below...
Is a lot less readable than the code before it. "db" is a crucial part of explaining what's going on. What's a "connection"?
Comment #5
dawehnerSo connection seems to be more consistent
Comment #6
dawehnerOn the other hand though we do have \Drupal::database() and nothing on ControllerBase()
Comment #7
damiankloip CreditAttribution: damiankloip commentedI don't really mind either way, doesn't really affect the readability in my eyes but here are those changes.
If this the new way to go, then we might want to start creating issues to convert the $connection usage. As #5 states the numbers, and $connection is more popular atm :)
Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedwell
Comment #9
chx CreditAttribution: chx commented'connection' imo breaks encapsulation ; why do I need to know or care that the database system uses a connection object to communicate with the database?
'connection' is a bad name because it does not tell me what am I connected to.
So, 'database' is a win.
Comment #10
damiankloip CreditAttribution: damiankloip commentedOk great, then looks like this will be the new 'thing' then.
Comment #11
dawehnerthis should be $database
Comment #12
damiankloip CreditAttribution: damiankloip commentedGood spot.
Comment #13
dawehnerAnd it is still ready to go!
Comment #14
alexpottPatch no longer applies.
Comment #15
damiankloip CreditAttribution: damiankloip commentedLooks like this was already committed.
Comment #16
alexpottYep you're correct - committed 4e1d667
Comment #17
mcrittenden CreditAttribution: mcrittenden commentedTags