API page : https://api.drupal.org/api/drupal/core%21lib%21Drupal.php/function/Drupa...

Work required

* Remove reference to incorrect namespace \Drupal\Core\Routing\LinkGeneratorInterface::generate() in docblock for method \Drupal::l().

Original description

Wrong fully qualified class name \Drupal\Core\Routing\LinkGeneratorInterface::generate() is used in docs.
Since documentation page is already having correct @see \Drupal\Core\Utility\LinkGeneratorInterface::generate()
So changing this to LinkGeneratorInterface::generate() will be sufficient in comments.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

shashikant_chauhan created an issue. See original summary.

shashikant_chauhan’s picture

Status: Active » Needs review
FileSize
516 bytes

adding patch.

Chi’s picture

Version: 8.3.x-dev » 8.2.x-dev
Status: Needs review » Reviewed & tested by the community
xjm’s picture

Status: Reviewed & tested by the community » Needs work

I actually do not think we should use a shorthand here, because \Drupal is a mapping for so many different classes in so many different namespaces. Elsewhere in /core/lib/Drupal.php, we always use the fully qualified namespace for clarity.

However, I actually think the whole sentence is just redundant with the @see and the sentence before that says " the link generator service's generate() method". Let's just delete the sentence?

Also, @Chi, can you add a comment describing how you reviewed issues when you mark them RTBC? This will ensure you get credit for your reviews in the future, and also help committers know what points of review have been considered or not.

Thanks @Chi and @shashikant_chauhan!

shashikant_chauhan’s picture

Status: Needs work » Needs review
FileSize
559 bytes

@xjm, I have removed the redundant statement in this patch.
The issue & patch 2822296-2.patch was based on alexpott's comment https://www.drupal.org/node/2819479#comment-11748024

jp.stacey’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
477 bytes

Just for clarity:

* I've reworded the description for this ticket to talk about removing the reference rather than rewording it, to match @xjm's request in #4 and @shashikant_chauhan's patch in #5.
* I'm also uploading an interdiff for patches #2 and #5, to get some idea of what's changed.

Otherwise, I can confirm the patch applies cleanly to 8.3.x HEAD and satisfies the new description.

jp.stacey’s picture

Sorry: I did apply to 8.3.x HEAD rather than 8.2.x HEAD; but do we not want to be applying it to the most up-to-date 8.* HEAD, and only backporting it if we need to?

  • xjm committed 38959d6 on 8.3.x
    Issue #2822296 by shashikant_chauhan, jp.stacey, xjm: Wrong fully...

  • xjm committed b062c68 on 8.2.x
    Issue #2822296 by shashikant_chauhan, jp.stacey, xjm: Wrong fully...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @jp.stacey and @shashikant_chauhan!

@jp.stacey, documentation improvements can always be backported, so this was moved to 8.2.x to indicate it would be committed to both active branches. Typically it's okay to create and test backportable patches with either branch, except for the rare cases where the branches have diverged and the patch doesn't apply to both. You can see the details of what patches we backport at: https://www.drupal.org/core/d8-allowed-changes#patch

I confirmed this is the only case of this stale reference in core:

grep -r "Routing\\\\LinkGeneratorInterface" *
core/lib/Drupal.php:   * \Drupal\Core\Routing\LinkGeneratorInterface::generate().

Committed and pushed to 8.3.x and 8.2.x. Thanks!

jp.stacey’s picture

@xjm thanks for the clarification!

Status: Fixed » Closed (fixed)

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