Code review comment for lp:~futatuki/mailman/i18n-add-whence-to-adminack-templates

Mark Sapiro (msapiro) wrote :

I'm going to merge this although somewhat differently. I like what you have done and it is conceptually similar to what we do in Mailman 3. There, we have augmented i18n._ with a context manager to not do the translation and then we can do things like:

    with _.defer_translation():
        # This will be translated at the point of use.
        msgdata.setdefault('moderation_reasons', []).append(
            _('Message contains administrivia'))

to mark messages without translating them. However, there are already many places in Mailman 2.1 where we accomplish a similar thing by doing things like:

    def _(s):
        return s

    _('string to translate later')

    _ = i18n._

In fact, for example, in Mailman/ we already have

def _(s): return s

REASONS = {MemberAdaptor.BYBOUNCE: _('due to excessive bounces'),
           MemberAdaptor.BYUSER: _('by yourself'),
           MemberAdaptor.BYADMIN: _('by the list administrator'),
           MemberAdaptor.UNKNOWN: _('for unknown reasons'),

_ = i18n._

To to be more consistent with what is done elsewhere in the code, rather than defining D_() in i18n, I'll define it in the places it's needed and then just do things like

    _ = D_

    _('string to translate later')

    _ = i18n._

Your way is actually better, but in this case, I think it's more important to maintain consistency

review: Approve

« Back to merge proposal