the getUrlFromPath method in TwigExtension is deprecated. So let's remove it here.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
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.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JeroenT’s picture

Issue tags: +@deprecated, +Novice

Status: Needs review » Needs work

The last submitted patch, remove_url_from_path.patch, failed testing.

Status: Needs work » Needs review

umarzaffer queued remove_url_from_path.patch for re-testing.

umarzaffer’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs beta evaluation
umarzaffer’s picture

Issue tags: +SrijanSprintDay
xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record updates

Nice 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 blame on the line with the @deprecated to 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!

JeroenT’s picture

Issue summary: View changes
JeroenT’s picture

Issue tags: -Needs beta evaluation
JeroenT’s picture

umarzaffer’s picture

@xjm, noted.

JeroenT’s picture

Status: Needs work » Needs review
cilefen’s picture

It needed a reroll.

cilefen’s picture

@JeroenT Since this is a twig extension, I would make it its own change record.

cilefen’s picture

Oh, maybe #9 is the way to go, considering #6.

JeroenT’s picture

cilefen’s picture

Would it be appropriate to add twig-based example to the change record?

JeroenT’s picture

@cilefen,

Something like this? :

Creating an URL from a route name:

{{ url('<front>') }}

Creating an URL from a route name with parameters.

{{ url('entity.user.edit_form', {'user': '1'}) }}
cilefen’s picture

Also do the "before this change" section.

JeroenT’s picture

@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'}) }}

JeroenT’s picture

Made 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'}) }}

Mile23’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs a reroll.

Helpful how-to here: https://www.drupal.org/patch/reroll

cilefen’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.95 KB
cilefen’s picture

Re #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

Mile23’s picture

Status: Needs review » Reviewed & tested by the community

Yay green. :-)

The single-purpose change record is better, too.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed bf5ee20 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation the issue summary.

  • alexpott committed bf5ee20 on 8.0.x
    Issue #2493697 by cilefen, JeroenT: remove deprecated getUrlFromPath...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.