Problem/Motivation

The method marked internal and has no usage in contrib http://codcontrib.hank.vps-private.net/search?text=generateFromLink&file...

There's only one place in core where it used but using generate() method instead will save a function call per each each rendered link on page

Proposed resolution

deprecate the method and use generate() instead

Remaining tasks

- patch
- review
- commit

User interface changes

no

API changes

LinkGeneratorInterface::generateFromLink() is deprecated

Data model changes

no

Release notes snippet

CommentFileSizeAuthor
#6 3342991-6.patch3.24 KBquietone
#6 interdiff-2-6.txt2.67 KBquietone
#2 3342991-2.patch3.25 KBandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 3342991-2.patch3.25 KB

Filed CR and patch, not sure if deprecation needs separate method as generate() also should become internal

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

I think this needs to point to Link::fromTextAndUrl() for the alternative so it's not @internal. But otherwise looks good.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Link.php/...

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

It can't point to Link::fromTextAndUrl() because this method returns GeneratedLink object but Link is renderable element

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Oh now I see... this is what gets called by Link::toString().

So all we're doing is removing an internal wrapper for another internal method, it's slightly odd that the deprecation points there, but in practice people should already not be using this, so they won't see it, and if they are, then worst thing is they use ::generate() instead which will be no worse than they're already doing.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-2-6.txt2.67 KB
new 3342991-6.patch3.24 KB

The deprecation message for a method does not start with 'The'. And I was getting a phpcs error on the deprecation message in the interface.

I updated the patch.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

My bad, thank you for fix

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 10.1.x, thanks!

  • longwave committed 88834467 on 10.1.x 
    Issue #3342991 by quietone, andypost, catch: Deprecate...

Status: Fixed » Closed (fixed)

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