Page MenuHomeWildfire Games

Improvements to the translators credits script
Needs ReviewPublic

Authored by Itms on Apr 2 2019, 11:12 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Tiber7 reported that their name had disappeared from the credits. Upon investigation, this is due to a bug in the regex parsing the translator names.

I used the opportunity to also:

  • Port the script to Python 3
  • Remove a broken feature of the script that has been unused since the introduction of the translation credits
  • Remove trailing whitespace in the credits file (not included in the current diff, for clarity)
Test Plan

Test the script and check the changes performed on the credits list are consistent with the result I included in the patch

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
SeverityLocationCodeMessage
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:2TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:5TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:19TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:59TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:62TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:65TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:72TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:79TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:85TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:127TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:148TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:157TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:182TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:307TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:311TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:314TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:320TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:389TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:410TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:431TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:455TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:482TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:506TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:663TXT6Trailing Whitespace
Auto-Fixbinaries/data/mods/public/gui/credits/texts/translators.json:721TXT6Trailing Whitespace
Unit
No Unit Test Coverage
Build Status
Buildable 7082
Build 11565: Vulcan BuildJenkins
Build 11564: arc lint + arc unit

Event Timeline

Itms created this revision.Apr 2 2019, 11:12 PM
Itms added a comment.Apr 2 2019, 11:19 PM

Some inline details.

binaries/data/mods/public/gui/credits/texts/translators.json
2

The whitespace fix was excluded from the diff for clarity (it changes one third of all lines in that file).

source/tools/i18n/creditTranslators.py
78

Removed broken feature

83

This now matches names containing non alphanumerical chars (typically, dashes and dots). I excluded < and , which respectively mark the email address boundaries and the years of activity.

83

Unicode is the Python 3 default

108
113

Ditto

118

Python 3 removes some whitespace by default. In order to keep the same behavior as previously and obtain the current diff, use the option separator=(', ', ': ').

Vulcan added a subscriber: Vulcan.Apr 2 2019, 11:22 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1152/display/redirect

bb added a subscriber: bb.Apr 5 2019, 12:23 PM
bb added inline comments.
binaries/data/mods/public/gui/credits/texts/translators.json
2

sure, but as the added lines have trailing whitespaces aren't we adding them with the script, thus the next time we run it, we need to remove them again?

186

Where did he went?

310

names of ppl nowadays, thought twice the same character was weird enough...

497

could be name changes too, but not clear seeing from this file...

566

same

917

same

1676

Actually I don't believe this: much more logical would be that this guy actually is S\u00e1ndor Uszkai who is removed below

2096

Never seen him

2472

k, name change

3024

don't see him back either

3318

same

source/tools/i18n/creditTranslators.py
106

Is that still valid as we don't pre-add existing ones anymore?

Itms marked 2 inline comments as done.Apr 7 2019, 7:50 PM

@bb Thanks for the remarks! Regarding the name disappearing or changing too stupidly:

  • If users want to use very strange nicknames on Transifex, we can't do a lot.... It's worth noting that in Transifex the username is used by default, but you can choose to supply a different name to appear in the po files ("Nicolas Auvray" instead of "Itms" typically). So some of the changes might be from people deleting this extra information.
  • If all translations of a user have been overwritten over time, they will disappear from the po file. If that is the case for all resources, we will loose trace of them. I agree that in theory we should keep them in the credits (for code or art, even if all changes from someone are superseded later, we thank them for contributing). However, we have no possibility to automatically detect if a name removal is a name change, or all translations from a user obsolete. So we would have, for instance, both Nicolas Auvray and Itms in the credits (stupid). There is also a privacy issue: if someone wants to remove their real name from the credits, they will do so on Transifex, but they won't be warned that we keep old names in the credits, and they won't know they should contact us.

So I think it's better to "forget" about people who contributed obsolete translations. If they want to be in the credits, we can easily see in the SVN log that they indeed contributed, and add them manually. Doing the converse and removing manually all old names of translators would be too much work.

binaries/data/mods/public/gui/credits/texts/translators.json
2

No, when I commit the script, I'll commit the updated json file with all the whitespace removed (including in lines that were not changed here).

2096

Very fishy: he seems to be in several language credits ๐Ÿค”

source/tools/i18n/creditTranslators.py
106

Yes, as there will be names in each translation resource file for each language, thus always creating duplicates.

bb added a comment.Apr 7 2019, 10:12 PM

I get that ppl can choose whatever name they want, but who is named 4c905de7e2c9950b7d83273a8070b072 (second one in the german list)? I tried finding him/her in transifex but no luck.

Regarding the removing: I guess that would work, but it needs to be stated in a transifex privacy policy I suppose

binaries/data/mods/public/gui/credits/texts/translators.json
2

wasn't my question: is the script adding whitespaces?

source/tools/i18n/creditTranslators.py
106

k