Now that the instance storage of the controllers has changed. We have alot of duplication. We can refactor this out to just call a generic getController method.

CommentFileSizeAuthor
#15 2001012-15.patch3.41 KBdamiankloip
#7 2001012-7.patch3.41 KBdamiankloip
#4 2001012-4.patch2.45 KBdamiankloip
#4 interdiff-2001012-4.txt796 bytesdamiankloip
#2 2001012-2.patch2.99 KBdamiankloip
d8-EntityManager-refactor.patch3.93 KBdamiankloip

Comments

Status: Needs review » Needs work

The last submitted patch, d8-EntityManager-refactor.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2001012-2.patch2.99 KB

How about we roll this against the current HEAD, not the one I'm expecting in the future.

Status: Needs review » Needs work

The last submitted patch, 2001012-2.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2001012-4.txt796 bytes
new 2001012-4.patch2.45 KB

Forget about the list controller needing the storage controller injected.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/lib/Drupal/Core/Entity/EntityManager.phpundefined
@@ -193,11 +185,26 @@ public function getRenderController($entity_type) {
+  protected function getController($entity_type, $controller_type) {

Just by random guess I would have expected to have the controller type as first parameter/

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

This certainly needs a full reroll, due to the injection issue.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2001012-7.patch3.41 KB

rerolled.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Great. Thank you!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2001012-7.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review

#7: 2001012-7.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2001012-7.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review

#7: 2001012-7.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

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

Needs a reroll...

curl https://drupal.org/files/2001012-7.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  3493  100  3493    0     0   4035      0 --:--:-- --:--:-- --:--:--  5244
error: patch failed: core/lib/Drupal/Core/Entity/EntityManager.php:204
error: core/lib/Drupal/Core/Entity/EntityManager.php: patch does not apply
damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2001012-15.patch3.41 KB

Rerolled.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Another one!

alexpott’s picture

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

Committed 9dabd6d and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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