This is an to issue to catch various slightly off-topic or follow up sub-issues from the initiative to clean up core by removing t() from test assert messages.

CommentFileSizeAuthor
#7 1797940-7-language-test-doc.patch841 bytesdcam
#1 follow-up-1797940-1.patch1.43 KBlars toomre

Comments

lars toomre’s picture

lars toomre commented
StatusFileSize
new follow-up-1797940-1.patch1.43 KB

This is follow up to review of #1797360: Remove t() from assertion messages in tests for the language module. LanguageUrlRewritingTest::checkURL() unexpectedly used two assert messages and the helper function in that test did not have any @param directives. The attached patch rectifies that.

lars toomre’s picture

lars toomre commented
Status: Active » Needs review

Helps to set the right status!

lars toomre’s picture

lars toomre commented

In reading through many test files, I have also noticed inconsistent formatting as well as the use of t() in getInfo() functions. This is invariance with http://drupal.org/node/325974 and should be addressed once the bulk of the remove t() assert issues have been committed.

xjm’s picture

xjm
she/her
Primary language English
commented

Rather than filing one followup, each unrelated change should have its own issue with a descriptive title. So the docs fix in #1 for example can be filed as an independent documentation issue, and removing t() from getInfo() should be another separate issue, etc.

lars toomre’s picture

lars toomre commented

OK @xjm... I find it helpful to catch all for notes while doing the primary issue and then when have a little more time, to move them out to full follow up issues.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: Follow up to remove t() from test asserts » Fix docs for helper method in language tests
Version: 8.x-dev » 7.x-dev
Component: base system » documentation
Status: Needs review » Patch (to be ported)

Yes, each follow-up needs its own issue. Fixing the title for this one, and I've committed it to 8.x. Presumably this method also exists in 7.x somewhere and this needs to be ported?

dcam’s picture

dcam commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 1797940-7-language-test-doc.patch841 bytes

Backported #1 to D7.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

Thanks! I'll get this committed ASAP.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Committed to 7.x -- thanks again!

Status: Fixed » Closed (fixed)

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