- Issue created by @alexpott
- Status changed to Needs review
about 1 year ago 9:37am 8 February 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Making this major as it is part of π Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller Active
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Literally zero remarks on the MR.
All this needs now is a change record (for the few souls that have a custom
DrupalKernelInterface
implementation).I'd also like to see a test-only run, but I can't trigger that β could you please do that, @alexpott? π
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Tagging for blocking π Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller Active .
- π¬π§United Kingdom alexpott πͺπΊπ
Created a CR and triggered a test only run.
- Status changed to RTBC
about 1 year ago 11:30am 8 February 2024 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Test-only job reports:
There was 1 error: 1) Drupal\Tests\Component\DependencyInjection\ContainerTest::testReset Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "service_container".
π π’
- πΊπΈUnited States dww
Agreed that this looks great. Almost nothing to complain about in the MR, title, summary, or the CR. Really excited about the issue this is blocking! π
I opened a few MR threads for optional questions to address, but this is ready as-is. +1 RTBC.
Thanks!
-Derek - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Could you trigger the test-only job again now, to ensure that also after addressing @dww's review the test coverage still does what we think it does? π
- Status changed to Needs work
about 1 year ago 6:05am 12 February 2024 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Left a couple of questions
- Status changed to Needs review
about 1 year ago 1:13pm 12 February 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Addressed @larowlan's feedback - thanks for the review!
- Status changed to Needs work
about 1 year ago 1:20pm 28 February 2024 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.
- Status changed to Needs review
about 1 year ago 1:41pm 28 February 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Fixed MR. It'd be great to land this so I can back to the issue this blocks.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Tried to do a thorough review here, because would love to see the blocked issue unblocked, but β¦ IDK enough about this part of core to RTBC this. Hopefully @larowlan does another review π€
- Status changed to RTBC
about 1 year ago 3:07am 29 February 2024 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Thanks for adding the new test @alexpott - I think this is good to go now.
- π¬π§United Kingdom alexpott πͺπΊπ
Merged 11.x and resolved use statement conflict in core/lib/Drupal/Core/DrupalKernelInterface.php due to π Remove ContainerAwareInterface from DrupalKernelInterface Fixed
Opened a 10.3.x MR so we can backport this there. - Status changed to Fixed
about 1 year ago 10:48am 29 February 2024 - π¬π§United Kingdom catch
Also can't find anything to complain about and the other issue will prove this is doing what it's supposed to on top of the extra test coverage here.
Committed/pushed to 11.x and 10.3.x respectively, thanks!
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Published the CR
- Status changed to Needs review
about 1 year ago 10:05pm 29 February 2024 - Status changed to Fixed
about 1 year ago 10:06pm 29 February 2024 Automatically closed - issue fixed for 2 weeks with no activity.