Problem/Motivation

Follow up of #3506931: Deprecate passing $root to \Drupal\Core\Database\Connection::createConnectionOptionsFromUrl.

Deprecate passing $root to \Drupal\Core\Database\Database::convertDbUrlToConnectionInfo() - since \Drupal\Core\Database\Connection::createConnectionOptionsFromUrl() no longer needs it we no longer need it at all.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3522513-nr-bot.txt4.68 KBneeds-review-queue-bot

Issue fork drupal-3522513

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

longwave created an issue. See original summary.

mondrake made their first commit to this issue’s fork.

mondrake opened merge request !12028

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented

Changes made

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs change record updates

Looks good to me, tagging for change record updates as it does make sense to merge this with the other one - let's do that on commit.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3522513-nr-bot.txt4.68 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Reviewed & tested by the community

The bot is wrong.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Actually no, something has changed in PHPStan, I get the same locally, though the PHPStan CI job passes?

  Line   lib/Drupal/Core/Test/TestSetupTrait.php (in context of                                           
         class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39)                          
 ------ ------------------------------------------------------------------------------------------------- 
  137    Method                                                                                           
         class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39::prepareDatabasePrefix()  
         has no return type specified.                                                                    
         🪪  missingType.return                                                                           
  146    Method                                                                                           
         class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39::changeDatabasePrefix()   
         has no return type specified.                                                                    
         🪪  missingType.return
mondrake’s picture

mondrake
Location 🇮🇹
commented

Looks like bot and your local have an old baseline version? I added the return type and the core CI job tells me to remove the above from the baseline

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review

https://git.drupalcode.org/issue/drupal-3522513/-/pipelines/490932

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems phpstan is still failing.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Tests seem to be happy now

@longwave you tagged for change record updates?

catch made their first commit to this issue’s fork.

  • catch committed 5d589fbe on 11.x 
    Issue #3522513 by mondrake, longwave: Deprecate passing $root to \Drupal...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs change record updates

Updated the previous CR to include this issue.

This isn't disruptive so can still be removed in 12.x, updated the deprecation version to 11.3

Committed/pushed to 11.x, thanks!

catch’s picture

catch
he/him
Primary language English
commented

catch closed merge request !12028

Status: Fixed » Closed (fixed)

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