Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
CivicActions is reviewing and upgrading multiple modules for use on client sites. Part of this work is a coding standards and i18n review. Attached you will find a patch based on a review with the coder module and a careful examination of the code.
The title and description of a hook_menu item don't need to be t()ed. More information can be found here: http://drupal.org/node/140311
Thanks!
Comment | File | Size | Author |
---|---|---|---|
#9 | nodecomment_st.patch | 1.16 KB | stella |
#7 | nodecomment_i18n.patch | 1.74 KB | stella |
#4 | nodecomment.patch | 980 bytes | wmostrey |
nodecomment.patch | 616 bytes | wmostrey |
Comments
Comment #1
wmostrey CreditAttribution: wmostrey commentedComment #2
catchPatch looks good and applies cleanly.
Comment #3
catchNot quite, there's also an unnecessary call to t() in _node_comment_delete_thread() - watchdog descriptions aren't passed through t() either now.
Comment #4
wmostrey CreditAttribution: wmostrey commentedHere's the updated patch.
Comment #5
catchThis looks great now.
Comment #6
sirkitree CreditAttribution: sirkitree commentedI've taken rc1 and applied that to HEAD so it can be worked on from there. Haven't had a lot of time to spend on this, but I applied this patch to HEAD as well. Thanks.
Comment #7
stella CreditAttribution: stella commentedI'm not sure those changes are being included in the 6.x-1.x-dev tarball. The changes were committed to HEAD, but the tarball appears to be generated from DRUPAL-6--1 branch, where other changes have also been made since. I'm pretty sure these i18n changes aren't included.
I've re-rolled the patch, so it includes the above 2 changes and one additional internationalization issue.
Cheers,
Stella
Comment #8
sirkitree CreditAttribution: sirkitree commentedsmall error in the patch:
should be:
committed to DRUPAL-6--1 branch.
http://drupal.org/cvs?commit=144468
Comment #9
stella CreditAttribution: stella commentedThat's great, thanks! However, I just noticed one other small error - one instance introduced by the patch and one that already existed. The
t()
function can't be used in install files as it may not always be available. Instead theget_t()
function should be used to determine whethert()
orst()
should be used. The attached patch fixes this.Cheers,
Stella
Comment #10
sirkitree CreditAttribution: sirkitree commentedah, interesting. good to know, thanks!
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.