As suggested by @Cottser on #500866: [META] remove t() from assert message, this is a new issue to pick up the views and views_ui remaining items from the t() clean-up in assert messages and groups.

CommentFileSizeAuthor
#7 removing_remaining_ts-2035087-7.patch17.74 KBlazysoundsystem
#7 removing_remaining_ts-2035087-interdiff.txt1.93 KBlazysoundsystem
#4 removing_remaining_ts-2035087-4.patch20.17 KBlazysoundsystem
#2 removing_remaining_ts-2035087-1.patch20.18 KBlazysoundsystem
#1 removing_remaining_ts-2035087-1.patch41.57 KBlazysoundsystem
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lazysoundsystem’s picture

lazysoundsystem CreditAttribution: lazysoundsystem commented
FileSize
removing_remaining_ts-2035087-1.patch41.57 KB

Here's the patch.

lazysoundsystem’s picture

lazysoundsystem CreditAttribution: lazysoundsystem commented
Status: Active » Needs review
FileSize
removing_remaining_ts-2035087-1.patch20.18 KB

Ignore the patch above - here's the same patch without the changes from #2035077: removing t() from asserts - remaining changes. but with Drupal\Component\Utility\String::format() instead of format_string(), as suggested there.

Crell’s picture

Crell CreditAttribution: Crell commented 
+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/SortDateTest.php
@@ -199,10 +200,10 @@ public function testDateOrdering() {
+        ), String::format('Result is returned correctly when ordering by granularity @granularity, @reverse.', array('@granularity' => $granularity, '@reverse' => $reverse ? t('reverse') : t('forward'))));

Still some t()s.

lazysoundsystem’s picture

lazysoundsystem CreditAttribution: lazysoundsystem commented
FileSize
removing_remaining_ts-2035087-4.patch20.17 KB

Thanks for the review - should be better now.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Reviewed & tested by the community

Thanks! Please remember to include interdiffs with patches. It makes it easier to review updates until we get on a real git-based workflow.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Reviewed & tested by the community » Needs work

This removal of t() is not correct:

-    $this->assertEqual($result[0]->attributes()->title, t('Page'));
+    $this->assertEqual($result[0]->attributes()->title, 'Page');

That is comparing a title to a translated string. It should have t() on it. It's not the message.

There are a couple of other spots like this in the patch.

lazysoundsystem’s picture

lazysoundsystem CreditAttribution: lazysoundsystem commented
Status: Needs work » Needs review
FileSize
removing_remaining_ts-2035087-interdiff.txt1.93 KB
removing_remaining_ts-2035087-7.patch17.74 KB

Good spot. Thanks. Here it is again without those, and with an interdiff.

Status: Needs review » Needs work

The last submitted patch, removing_remaining_ts-2035087-7.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Needs review

#7: removing_remaining_ts-2035087-7.patch queued for re-testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs review » Reviewed & tested by the community

Looks good! This takes care of everything from Views and Views UI.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Component: simpletest.module » views.module
Status: Reviewed & tested by the community » Fixed

Thanks! Committed to 8.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.