Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
See:
- #2089629: [Meta] Clean up DX issues found in porting modules from D7 to D8
- and conclusion notes from #HardProblems yesterday: https://docs.google.com/document/d/1RR-q-RPOXwC1j5w0p2cvHjqTStYcKh_n-nQx...
Comment | File | Size | Author |
---|---|---|---|
#11 | 2095587-remove-identifiableinterface-11.patch | 3.87 KB | dixon_ |
#12 | interdiff.txt | 571 bytes | dixon_ |
#3 | 2095587-remove-identifiableinterface-1.patch | 3.86 KB | dixon_ |
Comments
Comment #1
dixon_Tagging
Comment #2
dixon_Comment #3
dixon_Here we go.
Comment #4
dixon_Comment #6
jibranCan anyone please mention the reason here in the issue for the removal of
IdentifiableInterface
?Comment #7
jibran:/ x-post
Comment #8
Berdir#3: 2095587-remove-identifiableinterface-1.patch queued for re-testing.
Comment #9
BerdirThe 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.
This should now say The entity identifier I think.
Comment #10
BerdirComment #11
dixon_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.
Comment #12
dixon_Comment #13
BerdirOk, this looks good to me, thanks!
Comment #14
fubhy CreditAttribution: fubhy commentedYeah. +1 from me.
Comment #15
webchickYayyy!! :D Thanks, folks!
Committed and pushed to 8.x.
Comment #16
BerdirThere was no change notice for this being added, so we don't need to update any :)