Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Split off from #2416831: Add an active_theme twig function.
Problem/Motivation
The TwigExtension class has a setGenerators method which probably should have always been called setUrlGenerator because according to git log
that's all it ever injected.
Proposed resolution
Change it, or talk about a better way and do that instead.
Remaining tasks
Patch.
Patch review.
User interface changes
n/a
API changes
TwigExtension setGenerators will change to setUrlGenerator.
Beta phase evaluation
Issue category | Task, because its not a bug, it cleans things up |
---|---|
Issue priority | normal, because the positive impact is not dramatically. |
Prioritized changes | Correcting misspelled or wholly inaccurate method names is prioritized. |
Disruption | Not disruptive, its a internal method, and we are providing a BC-layer in case anyone else uses it. |
Comment | File | Size | Author |
---|---|---|---|
#10 | change_setgenerators_to-2496801-10.patch | 2.19 KB | cilefen |
#10 | interdiff.txt | 1.62 KB | cilefen |
#1 | change_setgenerators_to-2496801-1.patch | 1.79 KB | star-szr |
Comments
Comment #1
star-szrComment #2
star-szrComment #3
star-szrComment #4
tstoecklerOnce the beta evaluation is written, this is RTBC. This has confused me many times and I don't see any disruption caused by this change.
Comment #5
dawehnerI like the better name!
Comment #6
cilefen CreditAttribution: cilefen commented+1
If anyone is concerned enough about changing this method name, we could leave the existing method, marked deprecated, for BC.
Comment #7
star-szrThe only disruption might be if someone is extending core's TwigExtension, but 99.99% of the time they shouldn't be doing that anyway. https://www.drupal.org/node/1831138
Comment #8
tstoecklerRight and even then it's not likely (though certainly possible) that something will break.
Comment #9
alexpottLet's do this with a deprecation rather than removal - therefore nothing that extends TwigExtension will break. The lack of disruption is not a reason to commit something within beta. See the flowchart on https://www.drupal.org/core/beta-changes.
Renaming a misleading method name needs committer approval and should be done in a non-breaking way. I talked to @xjm and we approve of adding a new method
setUrlGenerator()
and the deprecation ofsetGenerators()
for Drupal 9.Also the beta evaluation needs fixing as the prioritized changes has not been completed.
Comment #10
cilefen CreditAttribution: cilefen commentedComment #11
cilefen CreditAttribution: cilefen commentedComment #12
star-szrInterdiff is misleading but patch looks good. Thanks @cilefen and thank you for setting us straight @alexpott!
Comment #13
dawehnerAlright, sheep it
Comment #15
alexpottCommitted f08e28a and pushed to 8.0.x. Thanks!
Thanks for adding the deprecation/bc layer and adding the beta evaluation to the issue summary.