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

Revert "Major German translation rework (01/12/2024)" #7628

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

sakertooth
Copy link
Contributor

@sakertooth sakertooth commented Dec 20, 2024

Reverts #7612

The German translation rework doesn't seem to be up to date with all the code changes that have transpired since 1.2.2. After some digging, it looks like they were intended for 1.2.2, so the PR set us a few steps back and broke CI.

Apologies for merging this too soon. Pinging @Wuzzy2 to let them know they should submit the necessary changes to Transifex instead, and after which we can pull them from there. They unfortunately might have to cherry-pick which translations are still applicable. For making translation updates, see here for more info.

@sakertooth sakertooth merged commit 6b494bd into master Dec 20, 2024
21 checks passed
@sakertooth sakertooth deleted the revert-7612-newde_master branch December 20, 2024 22:30
@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 21, 2024

👎 Lots of bad claims here.

  1. The PR you just reverted was !!!!NOT!!! for 1.2.2. That was from an older PR that I closed. The PR you reverted was for a recent commit. Read the PR text again
  2. You claim "it broke CI". This is very strange. What was the error message?
  3. You claim it "set us a few steps back". How?
  4. As I stated in the PR, I did extensive testing on a self-compiled version to verify the application uses German well. I did. I even ran lupdate myself to make sure the TS file is as up-to-date as it can possibly be. What tests did you make?
  5. Do you have any evidence for missing components in the de.ts file? (i.e. not just untranslated, but completely missing in the XML file) If yes, which ones?
  6. I will NOT use Transifex. It's proprietary software

@JohannesLorenz
Copy link
Contributor

Hi Wuzzy,

1. The PR you just reverted was !!!!NOT!!! for 1.2.2. That was from an older PR that I closed. The PR you reverted was for a recent commit. Read the PR text again

Try this:

git diff wuzzy2/newde_master upstream/stable-1.2 -- data/locale/de.ts | wc -l
git diff wuzzy2/newde_master upstream/master -- data/locale/de.ts | wc -l

These are just numbers, though. You will also see that the CI fails for exactly that reason, see below.

2. You claim "it broke CI". This is very strange. What was the error message?

In your PR #7612, CI was not run completely for a weird reason - Possibly because someone would have required to start the CI (it's not automatically started for first time contributers). It only ran 1 out of 10 jobs, and that one was randomly OK. However, the CI then ran on master, and on https://github.com/LMMS/lmms/commits/master/ you can clearly see how it introduced issues. You can check it even locally running the tests/scripted/check-strings script - it will show classes that were existing in 1.2.2, but then renamed/removed in 1.3 (e.g. git log -GBBTrack shows dc73911 which renamed that class).

3. You claim it "set us a few steps back". How?

See point 2. E.g. the PR adds translations for classes that do not exist anymore.

4. As I stated in the PR, I did extensive testing on a self-compiled version to verify the application uses German well. I did. I even ran lupdate myself to make sure the TS file is as up-to-date as it can possibly be. What tests did you make?

See point 2.

5. Do you have any evidence for missing components in the de.ts file? (i.e. not just untranslated, but completely missing in the XML file) If yes, which ones?

Possibly not relevant if Transifex ist used.

6. I will NOT use Transifex. It's proprietary software

FWIR, you can do it all using the Web UI? Of course, behind the Web UI might be proprietary software, but that's the same with Google, StackOverflow, GitHub etc.

The reason why we use that WebUI is that many non developers found it easier, and if we change translations from GitHub and Transifex at the same time, we will get conflicts. So there is no alternative.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 22, 2024

The components the CI complained about were all full of vanished and obsolete strings because I ran lupdate without -no-obsolete. These strings don't have an effect on the application, but it seems deleting those fixes the CI complaint. Keeping vanished and obsolete strings can be useful for future translators when a string changed so they don't have to start from 0 again. Anyway, I deleted them in the other PR to make CI happy.

@zonkmachine
Copy link
Member

  1. I will NOT use Transifex. It's proprietary software

But isn't GitHub proprietary? It's Microsoft owned.

I did have one complaint on merging translation files on GitHub. As I understand it files under data/locale/ are managed by our Transifex bot. Any manual changes here will be overwritten by the next Transifex updates. This means that if you don't wan't to migrate your stuff to Transifex, someone else will have to do it.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 23, 2024

@zonkmachine:

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.

Quoted from https://github.com/LMMS/lmms/wiki/Creating-a-localization. So is the wiki wrong? If yes, the wiki needs to be updated. If Transifex is absolutely mandatory and non-negotionable, then the above section doesn't make sense and should probably be deleted.

(Off-topic: I do a partial boycott of GitHub. None of my own projects are hosted here but I still use the PR and issue features of other projects. Why only partial? Because GitHub still works without JavaScript (more or less …). And simple HTTP requests don't require proprietary software on my side. Also, because I'm kinda forced to use GitHub by (ironically) several FOSS projects because that's basically the place where the important things happen. It's not my fault that so many FOSS projects depend on GitHub. I'm a big fan of Codeberg.org btw. Transifex, on the other hand, breaks down completely without JavaScript, so that's why I do a full boycott. The JavaScript is not the issue, it's that it's proprietary. Initially, I have discovered a workaround for Transifex, using some self-written shell scripts to use their API, but then their API was changed and I didn't care enough to update my scripts, so that's that.
Overall, the reason why I am so annoying about this is because I don't like the tendency of FOSS projects to inject proprietary software into the development workflow, DESPITE we already have high-quality FOSS replacements (Codeberg, Weblate). It seems like such a pointless betrayal of values.)

@JohannesLorenz
Copy link
Contributor

Quoted from https://github.com/LMMS/lmms/wiki/Creating-a-localization. So is the wiki wrong? If yes, the wiki needs to be updated. If Transifex is absolutely mandatory and non-negotionable, then the above section doesn't make sense and should probably be deleted.

@liushuyu Can you please comment on that?

@zonkmachine
Copy link
Member

@zonkmachine:

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.

Quoted from https://github.com/LMMS/lmms/wiki/Creating-a-localization. So is the wiki wrong? If yes, the wiki needs to be updated. If Transifex is absolutely mandatory and non-negotionable, then the above section doesn't make sense and should probably be deleted.

I understand that passage, with what follows on the same page, as meaning: You can choose not to use the Transifex online editor but instead use an editor like Qt Linguist. In the latter case you need to upload the resulting translation files separately in one action after finished work. With the online editor you submit each translation one by one as they're finished.
The text suggest mailing the translation file to a maintainer (lmms Transifex language maintainer for the language in question) if you can't upload it to your Transifex account.

I do agree that the documentation isn't very clear but my experience with working with Transifex online is that none of it is super clear. I personally just caved in to pushing strings online whenever I do Translation work. I don't like Transifex.

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.

5 participants