- 🇺🇸United States thomas.fleming
thomas.fleming → made their first commit to this issue’s fork.
- Status changed to Needs review
11 months ago 9:35pm 8 May 2024 - 🇺🇸United States thomas.fleming
I converted the patch to a merge request and also updated the call to entityManager to use entityTypeManager. Also confirmed that this is still an issue.
- Status changed to Needs work
11 months ago 10:59pm 8 May 2024 - 🇺🇸United States smustgrave
Thanks for reviving this one and verifying still an issue
Tagging for issue summary as it should follow the standard issue template.
Also tagging for tests since the change appeared to break a number of tests.
Thanks!
- First commit to issue fork.
- 🇪🇸Spain vidorado Logroño (La Rioja)
I believe this is a significant change that could cause WSODs after an update. Instead of throwing an exception, we should trigger a deprecation error first.
I've also fixed some failing tests caused by a core route that defined a route requirement for a
menu_link_content
entity—which has bundles—without specifying a bundle.Additionally, a change record has been issued. I will create a follow-up issue to replace the deprecation error with an
\InvalidArgumentException
. - 🇺🇸United States smustgrave
Can the CR be tweaked some. The code snippets are those suppose to be before/after?
- 🇪🇸Spain vidorado Logroño (La Rioja)
I tried to clarify them a bit :)
Thanks!
- 🇬🇧United Kingdom joachim
> I believe this is a significant change that could cause WSODs after an update. Instead of throwing an exception, we should trigger a deprecation error first.
Agreed, and if that's now the case, then this in the CR is wrong:
> The former will no longer be permitted. An exception will now be thrown if attempted.
- 🇪🇸Spain vidorado Logroño (La Rioja)
Ow, yes, you're right. I was thinking about Drupal 12, but the CR is for the Drupal 11.x minor version where this issue will be merged.
Sorry, I'm yet a CR rookie! xD