Files: 
CommentFileSizeAuthor
#11 2095587-remove-identifiableinterface-11.patch3.87 KBdixon_
PASSED: [[SimpleTest]]: [MySQL] 58,714 pass(es).
[ View ]
#12 interdiff.txt571 bytesdixon_
#3 2095587-remove-identifiableinterface-1.patch3.86 KBdixon_
FAILED: [[SimpleTest]]: [MySQL] 58,865 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

Issue tags:+Entity Field API

Tagging

Title:#HardProblems conclusion: Remove IdentifiableInterface#HardProblems DX conclusion: Remove IdentifiableInterface

StatusFileSize
new3.86 KB
FAILED: [[SimpleTest]]: [MySQL] 58,865 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here we go.

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, 2095587-remove-identifiableinterface-1.patch, failed testing.

Status:Needs work» Needs review

Can anyone please mention the reason here in the issue for the removal of IdentifiableInterface?

Status:Needs review» Needs work

:/ x-post

Status:Needs work» Needs review

The problem is that the separate interface hides the very important id() method for entities in a separate interface, this was pointed out by @webchick in a DX review.

The interface was added for param converting, but it turns out that it's not actually necessary, so the only place that uses it is the data reference stuff, where we already need a subclass anyway and one of the two implementations already has a overridden method anyway.

+++ b/core/lib/Drupal/Core/Entity/EntityInterface.php
@@ -42,6 +41,14 @@
+   *   The object identifier, or NULL if the object does not yet have an identifier.

This should now say The entity identifier I think.

Status:Needs review» Needs work

StatusFileSize
new3.87 KB
PASSED: [[SimpleTest]]: [MySQL] 58,714 pass(es).
[ View ]

This should now say The entity identifier I think.

Fixed this.

I've also been debugging the failing test. It passes just fine through the browser UI for me, but fails when running through Drush. It seems to be related to session and login handling somehow.

Status:Needs work» Needs review
StatusFileSize
new571 bytes

Status:Needs review» Reviewed & tested by the community

Ok, this looks good to me, thanks!

Yeah. +1 from me.

Title:#HardProblems DX conclusion: Remove IdentifiableInterfaceChange notice: #HardProblems DX conclusion: Remove IdentifiableInterface
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record, +Approved API change

Yayyy!! :D Thanks, folks!

Committed and pushed to 8.x.

Title:Change notice: #HardProblems DX conclusion: Remove IdentifiableInterface#HardProblems DX conclusion: Remove IdentifiableInterface
Status:Active» Fixed
Issue tags:-Needs change record

There was no change notice for this being added, so we don't need to update any :)

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