Replace drupal_container() with Drupal::service() in the layout module [#2011094]

Comment	File	Size	Author
#16	layout-2011094-16.patch	3.72 KB	kgoel

#16	interdiff.txt	1.75 KB	kgoel
#12	layout-2011094-12.patch	1.97 KB	kgoel

#12	interdiff.txt	1.96 KB	kgoel
#9	layout-2011094-9.patch	1.14 KB	kgoel

#7	layout-2011094-7.patch	1.96 KB	kgoel

#5	layout-2011094-5.patch	1.95 KB	kgoel

#2	layout-2011094-2.patch	1.97 KB	kgoel

Comment #1

kgoel CreditAttribution: kgoel commented 4 June 2013 at 02:19

Assigned:

Unassigned

» kgoel

Log in or register to post comments

File	Size
layout-2011094-2.patch	1.97 KB

Comment #3

4 June 2013 at 03:23

Status:

Needs review

» Needs work

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

Log in or register to post comments

Comment #4

ddrozdik CreditAttribution: ddrozdik commented 4 June 2013 at 07:40

+++ 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)

Log in or register to post comments

File	Size
layout-2011094-5.patch	1.95 KB

Comment #6

4 June 2013 at 15:52

Status:

Needs review

» Needs work

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

Log in or register to post comments

File	Size
layout-2011094-7.patch	1.96 KB

Comment #8

4 June 2013 at 19:04

Status:

Needs review

» Needs work

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

Log in or register to post comments

File	Size
layout-2011094-9.patch	1.14 KB

Comment #10

ddrozdik CreditAttribution: ddrozdik commented 8 June 2013 at 13:29

Status:

Needs review

» Reviewed & tested by the community

looks good - RTBC

Log in or register to post comments

Comment #11

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 15 June 2013 at 08:57

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');

Log in or register to post comments

File	Size
interdiff.txt	1.96 KB
layout-2011094-12.patch	1.97 KB

Comment #13

16 June 2013 at 12:06

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

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

Log in or register to post comments

Comment #14

kgoel CreditAttribution: kgoel commented 19 June 2013 at 18:15

Status:

Needs work

» Needs review

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

Log in or register to post comments

Comment #15

19 June 2013 at 19:39

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

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

Log in or register to post comments

Comment #16

kgoel CreditAttribution: kgoel commented 20 June 2013 at 21:18

Status:

Needs work

» Needs review

File	Size
interdiff.txt	1.75 KB
layout-2011094-16.patch	3.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;