drupal_container() is deprecated, and all calls in the layout module need to be replaced with Drupal::service(), except for where the module_handler service is requested, which needs to be replaced with Drupal::moduleHandler() (see #1957154: Replace calls to drupal_container()->get('module_handler') service with Drupal::moduleHandler())

CommentFileSizeAuthor
#16 layout-2011094-16.patch3.72 KBkgoel
#16 interdiff.txt1.75 KBkgoel
#12 layout-2011094-12.patch1.97 KBkgoel
#12 interdiff.txt1.96 KBkgoel
#9 layout-2011094-9.patch1.14 KBkgoel
#7 layout-2011094-7.patch1.96 KBkgoel
#5 layout-2011094-5.patch1.95 KBkgoel
#2 layout-2011094-2.patch1.97 KBkgoel

Comments

kgoel’s picture

kgoel commented
Assigned: Unassigned » kgoel
kgoel’s picture

kgoel commented
Status: Active » Needs review
StatusFileSize
new layout-2011094-2.patch1.97 KB

Status: Needs review » Needs work

The last submitted patch, layout-2011094-2.patch, failed testing.

ddrozdik’s picture

ddrozdik commented 
+++ b/core/modules/layout/layout.module
@@ -58,7 +58,7 @@ function layout_permission() {
  *   The layout plugin manager instance.
  */
 function layout_manager() {
-  return drupal_container()->get('plugin.manager.layout');
+  return Drupal::moduleHandler()->get('plugin.manager.layout');
 }

This is incorrect way, just use Drupal::service('plugin.manager.layout'); in this case

+++ b/core/modules/layout/lib/Drupal/layout/Plugin/Derivative/Layout.php
@@ -59,7 +59,7 @@ public function getDerivativeDefinitions(array $base_plugin_definition) {
-    foreach (drupal_container()->get('module_handler')->getModuleList() as $module => $filename) {
+    foreach (Drupal::moduleHandler()->get('module_handler')->getModuleList() as $module => $filename) {

Also incorrect using, should be: \Drupal::moduleHandler()->getModuleList()

In this part the leading \ should be before calling class(I just think we loosely agreed on omitting in procedural code)

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new layout-2011094-5.patch1.95 KB

Status: Needs review » Needs work

The last submitted patch, layout-2011094-5.patch, failed testing.

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new layout-2011094-7.patch1.96 KB

Status: Needs review » Needs work

The last submitted patch, layout-2011094-7.patch, failed testing.

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new layout-2011094-9.patch1.14 KB
ddrozdik’s picture

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

looks good - RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/layout/layout.moduleundefined
@@ -58,7 +58,7 @@ function layout_permission() {
 function layout_manager() {
-  return drupal_container()->get('plugin.manager.layout');
+  return Drupal::service('plugin.manager.layout');
 }

I'd be a fan of getting rid of this function entirely and replace calls to it with either the injected service (where possible) or Drupal::service('plugin.manager.layout')

+++ b/core/modules/layout/lib/Drupal/layout/Tests/LayoutDerivativesTest.phpundefined
@@ -33,7 +33,7 @@ public static function getInfo() {
-    $manager = drupal_container()->get('plugin.manager.layout');
+    $manager = \Drupal::service('plugin.manager.layout');

Should be $this->container->get('plugin.manager.layout');

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.96 KB
new layout-2011094-12.patch1.97 KB

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, layout-2011094-12.patch, failed testing.

kgoel’s picture

kgoel commented
Status: Needs work » Needs review

#12: layout-2011094-12.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Novice

The last submitted patch, layout-2011094-12.patch, failed testing.

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.75 KB
new layout-2011094-16.patch3.72 KB

@alexpott -I just ran grep on layout_manager and found that this is being called in LayoutController.php. I am not sure if you meant to get rid of this code also.

./lib/Drupal/layout/Controller/LayoutController.php: * @param \Drupal\layout\Plugin\Type\LayoutManager $layout_manager
./lib/Drupal/layout/Controller/LayoutController.php: function __construct(LayoutManager $layout_manager) {
./lib/Drupal/layout/Controller/LayoutController.php: $this->layoutManager = $layout_manager;

ddrozdik’s picture

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

Applied, works good.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 1230f1d and pushed to 8.x. Thanks!

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