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

Reapply "Major German translation rework (01/12/2024) (#7612)" (#7628) #7631

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Wuzzy2
Copy link
Contributor

@Wuzzy2 Wuzzy2 commented Dec 21, 2024

This reverts #7628, which reverted #7612, which updated German translations.
I believe there was nothing wrong with my German translation update and the revert was done in error, made for bad reasons. Please bring it back.

The main reasons given for the revert:

  1. My PR was for 1.2.2: False. It was made for commit 3562bbe, which is quite recent. I actually ran lupdate myself (see Major German translation rework (01/12/2024) #7612 for details) and tested it in-application. The revert PR did not provide testing or evidence for breakage
  2. "doesn't seem to be up to date". Just stated with zero evidence, not providing any missing string or component. My update seems up-to-date to me, according to my own tests in my compiled version. Evidence: Just compile it yourself and see
  3. "it broke CI". This was stated without evidence or explanation. Also, it doesn't make sense. The PR literally changed one XML file (that is valid according to my tests). I am confused as to how this could possibly affect CI

I just tested my PR again and it still works as expected.

If you believe there is anything wrong with the PR, please point out the errors. Because I don't see them.

Again, this was the lupdate line I used:

lupdate -locations none -I include plugins plugins/* plugins/*/*  src/* src/*/*  -ts data/locale/de.ts

If you're aware of a more "complete" lupdate line, please let me know. I think the lupdate line is the crux of this entire problem, because this is the one that collects all the strings. But even this line collects about 3200 strings and it looks quite complete to me in testing. Note the German translation is not 100% complete due to some very difficult strings that are hard to translate.

@kawogi @sakertooth

@JohannesLorenz
Copy link
Contributor

I now added my comment to #7628 .

Here, you can even see better how CI is failing ("checks / scripted-checks (pull_request)", click on Details).

Btw: "Wenn Sie daran interessiert sind, LMMS in eine andere Sprache zu übersetzen, oder eine bereits existierende Übersetzung verbessern möchten, können Sie uns gerne helfen!" -> It looks like your PR removes the comma after "sind" - I think it should not be removed (einleitender Nebensatz): https://chatgpt.com/share/676749de-c7c0-8010-9515-b516cae763bc .

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Dec 22, 2024

I just tried something based on a guess. I removed all vanished strings. Is the CI now fixed? Did I miss something?

The comma does exist in my translation, I don't understand. Also: Are we seriously using ChatGPT as a source now? o_O

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Dec 23, 2024

I believe CI should be happy now, so this might be ready for merge.

But I'm afraid the Transifex bot (or whatever mechanism you use) needs to be temporarily disabled before you merge this, to prevent changes from being overwritten after 2 seconds. (based on what a project member told me)
And then, after the merge, let Transifex import strings from the source. And then re-enabling the bot should be possible again.

(Ideally, I would really like it more if we didn't have to deal with proprietary Transifex in the first place … Anyway …)

To get the license question out of the way: I hereby grant everyone the right to use my work of the PRs #7612 and #7631 under CC0.

So if someone finds a way to include my work into LMMS in a different way, feel free to do so.

PS: Before anyone complains I am violating the workflow here, the wiki explicitly allows for my approach:

If you want to download the translation to work offline or don't want to use Transifex editor, you can work in our traditional way: First checkout a recent (stable) version of LMMS from Git repository (Accessing Git repository). Afterwards you need to create a new empty file called "XX.ts" where XX are two characters identifying your localization, e.g. de for Germany, fr for France, it for Italy and so on.

This is what I did in my original PR. If this wiki section is not true, actually, then please fix the wiki.

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

Successfully merging this pull request may close these issues.

2 participants