Closed (duplicate)
Project:
Drupal core
Version:
9.0.x-dev
Component:
config.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
29 Jan 2020 at 10:00 UTC
Updated:
23 Mar 2020 at 22:18 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 #5
hardik_patel_12 commentedKindly review a new patch.
Comment #6
hardik_patel_12 commentedComment #7
pratik_kambleComment #8
longwaveConfirmed the only remaining t() calls are in config.module and tests, so this is good to go.
Comment #9
longwaveComment #10
pratik_kamble+1 RTBC.
Comment #11
andypostFix one more place
Comment #12
andypostComment #13
longwaveGood spot on that untranslated string :)
Comment #14
alexpottcore/modules/config/tests/config_test/src/ConfigTestForm.php
\Drupal\config_override_test\Cache\PirateDayCacheContext::getLabel
\Drupal\config_override_integration_test\Cache\ConfigOverrideIntegrationTestCacheContext::getLabel
\Drupal\config_test\ConfigTestListBuilder::buildHeader
Let's update the test module too - people copy test modules so we should set good examples there.
Note leave the use of t() in *Test.php well alone there are other issues about that and it is complicated.
Comment #15
andypostFixed feedback #14 but now it's not clear about usage in
core/modules/config/tests/config_test/src/ConfigTestForm.phpthis form is supposed to be used in tests only so its translatable strings should not bleed into translationsComment #16
abhisekmazumdarComment #17
abhisekmazumdarThe patch looks and works fine for me.
Comment #18
alexpottI think we need to decide on the meta what to do here. We use the trait or we can extend ControllerBase. I'm not sure about what is better. I think that extending ControllerBas might future proof changes to these controllers that need normal controller functionality and don't want to jump through service injection hoops.
Comment #19
jungleAdjust patch letting ConfigController to extend ControllerBase, according to #18
Comment #20
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!