Closed (fixed)
Project:
Drupal core
Version:
8.9.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
29 Mar 2020 at 04:08 UTC
Updated:
24 May 2020 at 00:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
jungleDid not touch usages in tests.
Comment #4
kristen polAssigning to myself for review.
Comment #5
kristen polThanks for the patch.
1) Changes looks fine.
2) Patch applies cleanly to 8.9, 9.0, and 9.1.
3) Searching for
\Drupal::menuTree():I went through all of these to check if any could be changed to dependency injection but didn't see any.
4) Tests pass.
5) Marking RTBC.
Comment #9
xjmI confirmed that the menu tree service is already available in
MenuFormand that we're calling it correctly:I also confirmed that we still prefer
\Drupal::whateverService()in tests to injecting the services, because of container rebuilds etc. So that means:Since we don't have to inject the service anywhere new, this is a transparent change and can be backported during beta. (If we'd had to inject the service anywhere new, it'd be minor-only with a beta deadline.)
One could make the case that menu.api.php should give an example of using the service properly rather than assuming the example is for procedural or test code, but rewriting docs is a separate issue scope. Tagging "needs followup" for that.
Committed to 9.1.x, and cherry-picked to 9.0.x and 8.9.x. Thanks!
Comment #10
jungleThanks @Kristen Pol for reviewing, and Thanks @xjm for committing!
Follow-up filed, hoping that I understood #9.4 correctly.