Closed (fixed)
Project:
Drupal core
Version:
10.3.x-dev
Component:
other
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Oct 2022 at 09:16 UTC
Updated:
11 Aug 2024 at 13:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
rakesh.drupal commentedComment #3
quietone commented@rakesh.drupal, thanks for the patch.
The change of t() to $this->t is being addressed in #3113904: [META] Replace t() calls inside of classes . It is not being done on an individual basis because there are too many instances and there is a coder sniff for this. For example, there was an issue to convert all the t() functions in plugins. #3181778: [w/c September 17th] Replace t() with $this->t() in all plugins. I think what should happen here is that this is re-scoped to cover, say all Controllers. That is only a suggestion, testing by changing the phpcs.xml rule as is shown in the Issue Summary of #3181778: [w/c September 17th] Replace t() with $this->t() in all plugins would help to determine a sensible scope. This can then be a child of the meta.
Comment #5
avpadernoComment #6
kunalgautam commentedPatch #2 is working fine for Drupal 10.1.x version as well.
Comment #7
avpadernoWhat requested in comment #3 hasn't been done. This means the status is still Needs work.
Comment #8
ravi.shankar commentedWorking on this.
Comment #9
ravi.shankar commentedReplaced all the
t()with$this->t()in all the controllers.Added a patch please review.
Comment #10
spokjeLooks like not all Controllers that are in patch #9 use the
StringTranslationTraitor extend a class that does.At the very least: TestBot is not happy.
Also, if we go the replace-in-controller-only route in this issue, we need an update of the Issue Summary and title to make that clear.
Comment #11
chaitanyadessai commentedReplaced all the t() with $this->t()
Please review.
Comment #12
avpadernoComment #13
avpadernoAlso, the issue summary needs to be updated, since the issue has been rescoped.
Comment #14
quietone commentedI've been doing some more digging into the issues for fixing t() function. In this comment in the parent issue xjm outlines the scope for these issues. That means, that this issue needs to split into two issues, one for the Controllers that have the trait and one for the Controllers that do not have the trait.
There is also a sniff for this. To find the find that need to be changed I altered the DrupalPractice.Objects.GlobalFunction rule in phpcs.xml to the following.
The results show errors in 8 files, 5 of which extend from ControllerBase, which has the trait. Lets do those here. I have updated the IS with the files to be fixed here.
The ones that do not have the trait are
And are to be done in a separate issue.
Comment #17
quietone commentedComment #18
quietone commentedComment #19
quietone commentedComment #20
smustgrave commentedHiding patches for clarity.
Updated the summary to note that 2 of the findings aren't needed since those moved to contrib.
Reviewing the current code changes in the MR and change seems good
Comment #21
catchOne comment on the MR.
Comment #22
avpadernoComment #23
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #24
avpadernoComment #25
smustgrave commentedSeems straight forward to me and non disruptive
Comment #27
nod_Committed and pushed d2cf8e5268 to 11.x and ceeb8e0277 to 11.0.x and c550b71cd1 to 10.4.x and 0ce937d2f6 to 10.3.x. Thanks!
Comment #32
nod_