Problem/Motivation
Fix references to the deprecated Entity class. It was renamed to EntityBase.
During the search for references I've found some more inconsistencies, which I hope could be fixed together.
Steps to reproduce
Proposed resolution
Use this to search for instances grep -r "[/*].*[ \]Entity::" core | grep -v processStubRow | grep -v validateArgument | grep -v config_test
Remaining tasks
Review
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3094865
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3094865-fix-references-to
changes, plain diff MR !8756
Comments
Comment #2
hchonovComment #3
berdirthese unit tests also still reference the wrong class in @coversDefaultClass, which breaks them when removing the BC layer, currently fixing that in #3069696: Remove BC layers from the entity system.
Comment #4
hchonovOh, I've missed that one. But as Entity actually extends from EntityBase and does not override any methods it should be fine to adjust the comments that we are testing EntityBase methods right?
Comment #11
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.
At this time we would need a D10.1.x patch or MR for this issue.
Also probably will have to recheck the repo if there are other occurrences
Please do not just reroll
Comment #12
_utsavsharma commentedPatch for D10.1.x.
Comment #13
_utsavsharma commentedComment #14
smustgrave commentedhow did you check for other occurrences per #11
Comment #15
smustgrave commentedRemoving credit the patch has nothing to do with this ticket it seems.
Comment #16
smustgrave commentedComment #17
_utsavsharma commentedSorry my bad @smustgrave.
I will read the comments properly from now on.
Comment #18
smustgrave commentedStill need the D10 version of the patch.
Comment #19
_utsavsharma commentedPatch for 10.1.x.
Comment #20
nikhil_110 commentedPatch #19 Successfully applied... attached screenshot.
Comment #21
smustgrave commented@Nikhil_110 screenshot of the patch applying is not needed as the tests running show us that.
#19 the interdiff and patch are the same so not sure what was captured. Seems some size was lost from #2 if that could documented why please.
Comment #25
quietone commentedComment #26
quietone commentedThis is a bug because the documentation is incorrect. And thus, it is rather strict on making the minimal change to correct the documentation.
Comment #27
smustgrave commentedThank for the grep @quietone
MR appears to need manual rebase.
Comment #28
quietone commentedThanks, I rebased.
Comment #29
smustgrave commentedUsing the grep provided by @quietone believe all instances have been updated.
Comment #30
catchCouple of comments on the MR.
Comment #31
quietone commentedComment #32
berdirReviewed. Tagging this with "Novice" seems.. brave, lots of special cases here to think about.
Comment #35
quietone commented@Berdir, thanks for the review. I responded to all the comments.
Good point about the novice tag, so I will remove it.
Comment #36
berdirThanks @quietone, the adjustments looks good to me. I changed "a Action entity" to "an Action entity", think it's still OK for me to RTBC this then.
Comment #38
catchCommitted/pushed to 11.x, thanks!
Doesn't cherry-pick cleanly to 11.0.x, so just marking fixed.