Files: 
CommentFileSizeAuthor
#7 1797360-7-language-t.patch22.08 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,341 pass(es).
[ View ]
#1 language-1797360-1.patch26.75 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 41,699 pass(es).
[ View ]

Comments

Assigned:xjm» Unassigned
Status:Active» Needs review
StatusFileSize
new26.75 KB
PASSED: [[SimpleTest]]: [MySQL] 41,699 pass(es).
[ View ]

See the LanguageUrlRewritingTest::checkUrl() docs for that change. This patch includes multiple format_string() replacements.

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

Status:Needs review» Reviewed & tested by the community

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

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

Assigned:Unassigned» jhodgdon

Version:8.x-dev» 7.x-dev
Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Patch (to be ported)

Thanks! Committed to 8.x.

Status:Patch (to be ported)» Needs review
StatusFileSize
new22.08 KB
PASSED: [[SimpleTest]]: [MySQL] 40,341 pass(es).
[ View ]

Backported #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.

#7: 1797360-7-language-t.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

hi,
the patch seems good
Thanks

Status:Reviewed & tested by the community» Fixed

Thanks again - committed to 7.x.

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