Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

StatusFileSize
new2.33 KB

Kindly review a patch.

hardik_patel_12’s picture

Assigned: hardik_patel_12 » Unassigned
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3109743-2.patch, failed testing. View results

hardik_patel_12’s picture

StatusFileSize
new2.74 KB
new944 bytes

Kindly review a new patch.

hardik_patel_12’s picture

Status: Needs work » Needs review
pratik_kamble’s picture

Assigned: Unassigned » pratik_kamble
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed the only remaining t() calls are in config.module and tests, so this is good to go.

longwave’s picture

Component: configuration system » config.module
pratik_kamble’s picture

+1 RTBC.

andypost’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new631 bytes
new3.09 KB

Fix one more place

andypost’s picture

Parent issue: » #3113904: [META] Replace t() calls inside of classes
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Good spot on that untranslated string :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

core/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.

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new5.57 KB
new8.66 KB

Fixed feedback #14 but now it's not clear about usage in core/modules/config/tests/config_test/src/ConfigTestForm.php this form is supposed to be used in tests only so its translatable strings should not bleed into translations

abhisekmazumdar’s picture

Assigned: pratik_kamble » Unassigned
abhisekmazumdar’s picture

Status: Needs review » Reviewed & tested by the community

The patch looks and works fine for me.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/config/src/Controller/ConfigController.php
@@ -15,11 +15,13 @@
 use Drupal\system\FileDownloadController;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
 
 /**
  * Returns responses for config module routes.
  */
 class ConfigController implements ContainerInjectionInterface {
+  use StringTranslationTrait;
 

I 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.

jungle’s picture

Status: Needs work » Needs review
StatusFileSize
new8.85 KB
new1.13 KB

Adjust patch letting ConfigController to extend ControllerBase, according to #18

xjm’s picture

Status: Needs review » Closed (duplicate)

Thanks 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!