Closed (fixed)
Project:
Drupal core
Version:
10.1.x-dev
Component:
link.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
26 Aug 2022 at 12:04 UTC
Updated:
22 Jan 2023 at 22:59 UTC
Jump to comment: Most recent
Show commands
changes
Comments
Comment #2
dieterholvoet commentedComment #4
dieterholvoet commentedComment #6
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.
Reviewing the MR and the change looks good to me and can see this being very useful.
Only thing I see missing would be a change record indicating this new function.
Once that's added I can mark RTBC.
Comment #7
dieterholvoet commentedI added a draft change record. Reverting the commit and credit of @Abhijith S since the change is irrelevant (it actually introduces a CS issue) and the commit message is unrelated to the actual change. Seems like a low effort attempt to get issue credit.
Comment #8
smustgrave commentedthanks for the quick turnaround! Will wait for the tests to finish and mark it.
Comment #9
smustgrave commentedCI failed.
Comment #10
dieterholvoet commentedHuh, seems like @Abhijith S was right. I'm sorry for the false accusation, my bad! That's weird though, I thought the Drupal core convention was to write null in uppercase.
Comment #11
dieterholvoet commentedComment #12
smustgrave commentedThanks for the quick responses @DieterHolvoet
Everything looks good to me now!
Good job!
Comment #13
xjmThanks for working on this. I agree this would be a good API addition. I left a comment on the MR to add a return typehint. (Since it's a new method, parameter and return typehints are required).
Adding new methods to an interface is a BC break. However, since the interface has a 1:1 relationship with the
LinkItemclass, method additions are allowed in minor releases.Grepping contrib, there is one module that implements
LinkItemInterfacewithout extendingLinkItem:https://git.drupalcode.org/project/linked_entity_reference/-/blob/1.0.x/...
Their usecase for extending a different base class looks legit, so we could help them out and file an issue in that queue to add Drupal 10.1.x compatibility? I added a sentence to the CR to address their usecase.
Thanks!
Comment #14
xjmFixing attribution.
Comment #15
dieterholvoet commentedI added the return type and created the follow up issue: #3331248: Add a title getter to LinkedEntityReference.
Comment #16
smustgrave commentedConfirmed the 1 open thread has been resolved (but I can't resolve it)
Also see the follow up ticket was made for linked_entity_reference. See as they don't have a D10 release don't imagine that will be disruptive to anyone.
Comment #18
xjmOK this looks good now; nice addition. Committed to 10.1.x and published the change record. Thanks everyone!