Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Part of meta #500866: [META] remove t() from assert message.
Comment | File | Size | Author |
---|---|---|---|
#7 | 1797360-7-language-t.patch | 22.08 KB | dcam |
#1 | language-1797360-1.patch | 26.75 KB | xjm |
Comments
Comment #1
xjmSee the LanguageUrlRewritingTest::checkUrl() docs for that change. This patch includes multiple
format_string()
replacements.Comment #2
Lars Toomre CreditAttribution: Lars Toomre commentedI have gone through and reviewed all of the proposed message changes in this patch and they are accurate (including the suble last one!) With the test bot green, this is RTBC!
Regarding the last change, we really need to open up a follow-up issue to add @param directives to the method LanguageUrlRewritingTest::checkUrl(). At first glance, it is not at all intuitative that a test should have two messages as parameters two and three.
Thinking about this for a moment, I am going to open up a local branch to record some of these slightly off topic issues that we encounter in cleaning up t() assert messages. That then can be turned into an issue of its own afterwards.
Comment #3
Lars Toomre CreditAttribution: Lars Toomre commentedI created #1797940: Fix docs for helper method in language tests for follow-ups to the remove t() from assert messages. The first patch there in #1797940-1: Fix docs for helper method in language tests addresses the missing documentation mentioned in #2.
This patch is RTBC!
Comment #4
Lars Toomre CreditAttribution: Lars Toomre commentedI applied the patch in #1 and also went through looking at each assert message in the Tests for this module. I can confirm that no t() conversions were missed. Hence, this is RTBC!
Comment #5
webchickComment #6
jhodgdonThanks! Committed to 8.x.
Comment #7
dcam CreditAttribution: dcam commentedBackported #1 to D7.
#1797364: Remove t() from assertion messages in tests for the locale module will also affect t() functions in Locale. Because of that, I only backported the changes from D8's language module tests. I didn't alter anything else.
Comment #8
izus CreditAttribution: izus commented#7: 1797360-7-language-t.patch queued for re-testing.
Comment #9
izus CreditAttribution: izus commentedhi,
the patch seems good
Thanks
Comment #10
jhodgdonThanks again - committed to 7.x.