Closed (duplicate)
Project:
Drupal core
Version:
9.0.x-dev
Component:
node system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
16 Jan 2020 at 06:41 UTC
Updated:
23 Mar 2020 at 22:16 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
hardik_patel_12 commentedKindly review a patch.
Comment #3
hardik_patel_12 commentedComment #4
hardik_patel_12 commentedReplacing t() calls with in $this->t() in more files from node module.
Comment #5
hardik_patel_12 commentedComment #7
hardik_patel_12 commentedKindly review a new patch.
Comment #8
hardik_patel_12 commentedComment #9
longwave@Hardik_Patel_12: when you update a patch it is helpful to provide an interdiff against the previous patch so reviewers can easily see what was changed between the patches. See https://www.drupal.org/documentation/git/interdiff for documentation on this.
Comment #10
hardik_patel_12 commentedok sure.
Comment #11
hardik_patel_12 commentedInterdiff of #4 and #7
Comment #12
hash6 commentedComment #13
hash6 commentedThanks @Hardik_Patel_12 for the patch, reviewed and tested successfully.
Comment #14
swatichouhan012 commentedComment #15
alexpottThis is conflicting with a recent change. When re-rolling we need to ensure that the new and not the old text is used.
Comment #16
swatichouhan012 commented#16 patch failed to apply, i have rerolled a new one to remove all t calls, kindly review.
Comment #18
jungleWorking on this
Comment #19
jungle#15 checked
Comment #20
jungleShould move MigrationDeriverTrait and StringTranslationTrait to DeriverBase? Both D6NodeDeriver and D7NodeDeriver use the two traits.
Comment #21
xjmThanks for working on this.
In general, issues should not be scoped by file or module; instead, they should be scoped by making the exact specific change across as much of core as possible. Reference: https://www.drupal.org/core/scope#files
In particular,
t()calls should be replaced based on whether the translation service is already available in the class, and more specifically, based on which base class it extends. (So, for example, one issue for form builders, one for controllers, one for list builders, and then splitting that up further only if the resulting patch is too large to be manageable.) We also need to decide the approach before we proceed with child issues. See #3113904: [META] Replace t() calls inside of classes for more discussion. So, closing as a duplicate of the parent issue in #3113904: [META] Replace t() calls inside of classes .Thanks!