Page MenuHomePhabricator

l10n-bot may export empty translations to Git, making banana tests fail
Open, Needs TriagePublic

Description

The banana tests in most MediaWiki extensions fail the build if any translation texts are empty. However, those tests don’t run on @L10n-bot’s changes, so if the Translation updater bot imports empty translations from translatewiki.net, then subsequent builds for the same extension will suddenly start failing. This just happened in Wikibase (empty Hebrew translations added in I38770b6c09, gate-and-submit failed for I04cb13dbc4), but it’s also happened before, e. g. in T192201. It would be nice if l10n-bot would not import such messages and avoid breaking CI for developers.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 20 2020, 10:07 AM

Change 614719 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Manually import empty Hebrew translations

https://gerrit.wikimedia.org/r/614719

Lucas_Werkmeister_WMDE renamed this task from l10n-bot may import empty translations, causing banana tests to fail to l10n-bot may import empty translations, making banana tests fail.Jul 20 2020, 10:12 AM

Change 614719 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Manually import empty Hebrew translations

https://gerrit.wikimedia.org/r/614719

The naming convention is that changes are imported to translatewiki.net and translation updates exported (or pushed) to version control.

The linter for blank translations was not introduced by us and never discussed with translatewiki.net, so it is not clear to me why it was added and whether it is beneficial or not.

I see two options here to move this forward:

  • Remove that lint check
  • Someone explains the reasons for this lint check, and files a feature request against Translate&translatewiki.net to implement that check on our side and enforce it.
Lucas_Werkmeister_WMDE renamed this task from l10n-bot may import empty translations, making banana tests fail to l10n-bot may export empty translations to Git, making banana tests fail.Jul 20 2020, 2:32 PM

The linter for blank translations was not introduced by us and never discussed with translatewiki.net, so it is not clear to me why it was added and whether it is beneficial or not.

CC @Jdforrester-WMF and @Krinkle, who committed/authored that banana-checker rule (it was later enabled by default).

There is a setting you can disable this check on the repo, if needed.

See also T155182

It was five years ago, but I'm pretty sure we discussed it at the time, and it was based on the internals of how RL works.

Krinkle added a comment.EditedTue, Aug 25, 3:19 PM

There is no relation to MediaWiki or ResourceLoader from any of this as far as I'm aware in terms of it requiring or needing to have non-empty values.

I introduced disallowEmptyDocumentation because empty documentation is no documentation, and this was later enabled by default as it makes sense not to allow this for most projects.

James and I worked on disallowBlankTranslations together as well as a natural extension of that, so that if you work on something locally or have forgotten something and left an empty string that this is correctly detected as the most-likely mistake that it is. It is not clear to me how an empty string can be a valid source message nor a valid translation.

Now, focussing specifically on MediaWiki, it has the ability to disable messages. Not all features will support this, for those that do, the - (dash) is used as a way to communicate this our abstraction layers. Setting it to an empty string satisfies that as well, but is more ambiguous, and leaves the risk of something appearing to work when it doesn't. E.g. if a feature doesn't support disabling a message but you can set it to the empty string that would likely render an empty paragraph that still takes up space with other parts of the interface/feature/codebase thinking the message is there and relying on it.

In any case, if there are too many exceptions to this, presumably the rule is already disabled in those repos as otherwise CI would be failing. Given this came up recently through a new export only, I suspect this is not the case, but rather this might be something new or unusual.

Well, so far it seems all examples given about these have been actual issues, so it seems the check is useful.