Python3 l10n #2525
Python3 l10n #2525
Conversation
There was a problem hiding this comment.
Thank you very much @darkdreamingdan for going out your way to upgrade these scripts! This is much appreciated.
I am not fluent in Python so I can't really review the code so much, but it looks clear enough when I eyed it through, so I think it's fine. If there is anyone here fluent in Python, a code review would be appreciated.
I found some nitpick issues regarding some non-code related typos and such, but that's about it.
Code runs well on my Windows machine and it looks to be outputting correct stuff. I haven't extensively tested this (yet).
I wonder if this will run with our current GitHub Action out of the box. The goal would be to run these scripts either periodically, or on pull request and push, and create a new pull request for any source changes. This way the source pot remains up-to-date and our Crowdin project will automatically pull the latest source from Git. This is what our current GitHub Action does at the moment. I believe it is just a matter of upgrading the action's Python version to 3 and updating filepaths if needed. We could also move to Linux instead of Windows on the action for some added efficiency. This is all something I can have a look later in this pull request if that's fine with you, unless you would like to debug and upgrade the action yourself. Either is fine to me.
I think the only question from me now is that could we update the xgettext executable to the latest static version? I doubt we need the shared version (the one with the DLLs), those could be removed from our repo.
All in all great work! 👍
Update: I fixed the nit issues, updated the xgettext executable and rewrote the GitHub actions workflow to take pull requests into account.
Required by build_gettext_catalog_nsi.py
patrikjuvonen
force-pushed
the
python3_l10n
branch
from
374dcd2
to
16d0b95
Compare
patrikjuvonen
force-pushed
the
python3_l10n
branch
from
16d0b95
to
4b829cc
Compare
We've already got commits at this point, not staged changes. BRAINFAAART. [ci skip]
patrikjuvonen
force-pushed
the
python3_l10n
branch
from
18bdc2e
to
7bd115b
Compare
…any extra files [ci skip]
[ci skip]
Pull requests won't work too well as it requires someone to actively do something about them, causing pull requests and branches to go out of sync.
Running on pull_request causes conflicts
[ci skip]
[ci skip]
This PR refactors the Python l10n scripts. I've generally modernized them to be Python3 ready and written unit tests around them. Opening this for feedback.