Remove the weird init methods. They're just a hack to provide pseudo-documentation/autocomplete, and we're replacing it with methods.

CommentFileSizeAuthor
#22 kill-init-2095919-22.patch5.32 KBBerdir
#17 kill-init-2095919-17-interdiff.txt687 bytesBerdir
#17 kill-init-2095919-17.patch5.28 KBBerdir
#12 kill-init-2095919-12-interdiff.txt1.32 KBBerdir
#12 kill-init-2095919-12.patch5.23 KBBerdir
#6 kill-init-2095919-6.patch3.91 KBBerdir
#1 kill-init-2095919-1.patch6.34 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
kill-init-2095919-1.patch6.34 KB

This will fail until the content entities in #2016679: Expand Entity Type interfaces to provide methods, protect the properties have their init calls removed. I could do it here, but that would break the existing patches.

Status: Needs review » Needs work

The last submitted patch, kill-init-2095919-1.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Issue tags: +Entity Field API

Tagging.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review

1: kill-init-2095919-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: kill-init-2095919-1.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
kill-init-2095919-6.patch3.91 KB

Re-roll.

Status: Needs review » Needs work

The last submitted patch, 6: kill-init-2095919-6.patch, failed testing.

yched’s picture

yched CreditAttribution: yched commented

Would have happily RTBCed, but patch fails
(Testbot doesn't post fail notifications anymore ?)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Yeah, patch will fail until Comment and Term are updated, which is happening in #2028025: Expand CommentInterface to provide methods and #2016701: Expand TermInterface to provide methods. As commented in the meta issue, we could easily remove it all here, but the other issues make sure that we have proper method replacements.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review

6: kill-init-2095919-6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: kill-init-2095919-6.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
kill-init-2095919-12.patch5.23 KB
kill-init-2095919-12-interdiff.txt1.32 KB

Ah, I obviously need to remove the public langcode from the Entity class.

Status: Needs review » Needs work

The last submitted patch, 12: kill-init-2095919-12.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

12: kill-init-2095919-12.patch queued for re-testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review

Green!

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Needs review » Needs work 
+++ b/core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/Entity/EntityTest.php
@@ -174,4 +126,23 @@ public function setOwner(UserInterface $account) {
+  }

Minor: This should return $this as other setters.

Else, this is RTBC.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
kill-init-2095919-17.patch5.28 KB
kill-init-2095919-17-interdiff.txt687 bytes
aspilicious’s picture

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

Oh yes! I disliked this so much.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Title: Kill init() » Kill Entity::init()

I got confused multiple times, sorry.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: Kill Entity::init() » Kill ContentEntityBase::init()

Good point, let's be correct then :)

alexpott’s picture

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

kill-init-2095919-17.patch no longer applies.

error: patch failed: core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:10
error: core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Entity/ContentEntityBase.php:339
error: core/lib/Drupal/Core/Entity/ContentEntityBase.php: patch does not apply
error: patch failed: core/lib/Drupal/Core/Entity/Entity.php:16
error: core/lib/Drupal/Core/Entity/Entity.php: patch does not apply
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
kill-init-2095919-22.patch5.32 KB

Re-rolled.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Nice clean-up!

Committed and pushed to 8.x. Thanks!

  • Commit bbe2af1 on 8.x by webchick: 
    Issue #2095919 by Berdir: Kill ContentEntityBase::init() .

Status: Fixed » Closed (fixed)

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