the getUrlFromPath method in TwigExtension is deprecated. So let's remove it here.
| Issue category | Task. Work must be done to remove the deprecated getUrlFromPath method before 8.0.0 release. |
|---|---|
| Issue priority | Normal. Drupal 8 still works with this deprecated function still in place. |
| Prioritized changes | The main goal of this issue is removing the already deprecated getUrlFromPath method for 8.0.0. |
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | remove_deprecated-2493697-22.patch | 1.95 KB | cilefen |
| #12 | remove_deprecated-2493697-12.patch | 1.95 KB | cilefen |
| remove_url_from_path.patch | 1.95 KB | jeroent |
Comments
Comment #1
jeroentComment #4
umarzaffer commentedComment #5
umarzaffer commentedComment #6
xjmNice work! Since this is deprecated for 8.0.0 already, it's prioritized to remove it during the beta per https://www.drupal.org/core/beta-changes. (Keep in mind that's only code already deprecated for 8.0.0 before the beta.) It would be good to document this in the summary with a beta evaluation as @umarzaffer suggested with the issue tag.
We do need a change record for this as I didn't find an existing one. It would be best to update an existing change record since the method is already deprecated. There should already be a change record from the URL generation changes; try searching for that and even doing a
git blameon the line with the@deprecatedto look for issues that introduced it. Setting "Needs work" for that.Also a suggestion for @umarzaffer: When you set an issue to RTBC, you should add a comment explaining what you did to review the patch and why you believe it's RTBC. Thanks!
Comment #7
jeroentComment #8
jeroentComment #9
jeroent@xjm,
This is the issue where this method was marked deprecated: #2470569: Mark \Drupal\Core\Template\TwigExtension::getUrlFromPath() as deprecated.
How about adding it to this change record? Drupal URL generation is now done using routing system's UrlGenerator instead of url()
Comment #10
umarzaffer commented@xjm, noted.
Comment #11
jeroentComment #12
cilefen commentedIt needed a reroll.
Comment #13
cilefen commented@JeroenT Since this is a twig extension, I would make it its own change record.
Comment #14
cilefen commentedOh, maybe #9 is the way to go, considering #6.
Comment #15
jeroentCoupled this issue to the following change record: Drupal URL generation is now done using routing system's UrlGenerator instead of url().
Comment #16
cilefen commentedWould it be appropriate to add twig-based example to the change record?
Comment #17
jeroent@cilefen,
Something like this? :
Creating an URL from a route name:
Creating an URL from a route name with parameters.
Comment #18
cilefen commentedAlso do the "before this change" section.
Comment #19
jeroent@cilefen, thanks.
So I should add something like this?
Creating an URL from a route name:
before this change:
{{ url_from_path('user') }}After:
{{ url('user.page') }}Creating an URL from a route name with parameters.
Before this change:
{{ url_from_path('user/1') }}After:
{{ url('entity.user.edit_form', {'user': '1'}) }}Comment #20
jeroentMade changes to CR: Drupal URL generation is now done using routing system's UrlGenerator instead of url()
and added:
Creating an URL from a route name:
before this change:
{{ url_from_path('user') }}
After:
{{ url('user.page') }}
Creating an URL from a route name with parameters.
Before this change:
{{ url_from_path('user/1') }}
After:
{{ url('entity.user.edit_form', {'user': '1'}) }}
Comment #21
mile23Needs a reroll.
Helpful how-to here: https://www.drupal.org/patch/reroll
Comment #22
cilefen commentedComment #23
cilefen commentedRe #20: I made a new draft record for this change only because it didn't make sense to me at the end of the other one and I couldn't integrate it cleanly.
https://www.drupal.org/node/2556847
Comment #24
mile23Yay green. :-)
The single-purpose change record is better, too.
Comment #25
alexpottCommitted bf5ee20 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation the issue summary.