Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

StatusFileSize
new23.52 KB

Kindly review a patch.

hardik_patel_12’s picture

Assigned: hardik_patel_12 » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

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

hardik_patel_12’s picture

StatusFileSize
new23.91 KB
new1.26 KB

Kindly review a new patch.

hardik_patel_12’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 3109980-5.patch, failed testing. View results

hardik_patel_12’s picture

StatusFileSize
new20.4 KB
new3.67 KB

Kindly review a new patch.

hardik_patel_12’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: 3109980-8.patch, failed testing. View results

hardik_patel_12’s picture

The failing test isn't related to this patch.

Drupal\BuildTests\Framework\Tests\BuildTestTest::testPortMany
RuntimeException: Unable to start the web server.
ERROR OUTPUT:

hardik_patel_12’s picture

Status: Needs work » Needs review
mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

andypost’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new7.14 KB
new26.63 KB

Checked each file in image module and replaced ones which were missed

+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -389,7 +389,7 @@ public static function validateResolution($element, FormStateInterface $form_sta
-          $form_state->setError($element[$dimension], t('Both a height and width value must be specified in the @name field.', ['@name' => $element['#title']]));
+          $form_state->setError($element[$dimension], $this->t('Both a height and width value must be specified in the @name field.', ['@name' => $element['#title']]));

this method is static, so no t() accessible here, also in propertyDefinitions() more t() functions used

andypost’s picture

martin107’s picture

Status: Needs review » Reviewed & tested by the community

A) This patch still applies.
B) After searching - outside of the test directory, '*.inc' and *.module files there are no calls to t()
C) After a visual scan of the patch ... all changes are of the expected form
D) There are no unrelated changes.

E)
In other issues, of a similar form, but with a small number of changes I have recommended that we stop using t() within tests
as all tests are conducted in English unless directly testing string translation.

https://www.drupal.org/project/drupal/issues/3116080#comment-13481391

This is a large enough patch .. and the number of changes to tests is significant. Additionally progress on the meta issue has slowed.

So I am not going to recommend that here.

In short RTBC.

xjm’s picture

Status: Reviewed & tested by the community » 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!