Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running lupdate in dist.py is an anti-pattern #119

Open
GetPsyched opened this issue Dec 25, 2023 · 0 comments
Open

Running lupdate in dist.py is an anti-pattern #119

GetPsyched opened this issue Dec 25, 2023 · 0 comments

Comments

@GetPsyched
Copy link
Contributor

GetPsyched commented Dec 25, 2023

nexus/dist.py

Lines 69 to 72 in 3e6d11f

print("Generating TS templates...")
run_command(f"{venv_path}pyside6-lupdate " +
' '.join(glob.glob('ui/*.ui') + ["nexus/GUI.py"]) +
" -ts translations/i18n_en.ts")

This code essentially updates the i18n_en.ts file.

Since dist.py is meant to "setup" the project locally or to build it, it makes sense for it to generate files that may not exist in the repository (such as nexus/ui/BanlistDialog.py, etc.). However, it doesn't make sense to "update" existing files to match the code. This is because the code itself is expected to be up-to-date.

So, running pyside6-lupdate in dist.py either does nothing (if the files are up-to-date) or they update them to be accurate (this is bad since it means the file in repo is outdated)

Solution

I can think of 2 solutions:

  1. Don't track the translation files in Git and let them be generated by lupdate
  2. Don't update in dist.py, instead maybe do it exclusively in the pre-commit hook

Option 1 is less than ideal, since I suppose there are some manually defined things within those files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant