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

Add Korean translation #2685

Merged
merged 5 commits into from Jul 6, 2022
Merged

Add Korean translation #2685

merged 5 commits into from Jul 6, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jul 1, 2022

Short description of changes

This is creating translation_ko_KR.ts file for Korean.

CHANGELOG: Translation: Added Korean translation to Jamulus

Context: Fixes an issue?

#2653

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request

Ready for testing

What is missing until this pull request can be merged?
Testing

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see added this to the Release 3.9.0 milestone Jul 2, 2022
@ann0see
Copy link
Member

ann0see commented Jul 3, 2022

Hi! Great and big thanks for the translation. Around Line 108 there is a syntax error. You need to close the message tag there: </message>.
I'm not sure if we need to add anything to Jamulus.pro, but probably yes.

Did you already read https://github.com/jamulussoftware/jamulus/blob/master/docs/TRANSLATING.md?

@ghost
Copy link
Author

ghost commented Jul 3, 2022

OK, Thanks.

@ignotus666
Copy link
Contributor

Ok, no more errors ;) I can't comment on the Korean though...

@ann0see
Copy link
Member

ann0see commented Jul 3, 2022

Yes. That's probably up to the other Korean translator(s).
Next steps here:

  • Add Korean translation to Jamulus.pro if needed (done)
  • Get the compiled version from the checks tab and test if the Korean translation shows up correctly (done)
  • Add involved translators to the translators tab in the app
  • Add translators to GitHub organisation
  • Add invite to DC/Matrix

@ignotus666
Copy link
Contributor

@bagjunggyu I just opened your .ts file in QtLinguist and it shows a lot of untranslated strings, when they are actually translated. This is because the type="unfinished" tag is still there (it should be removed). Again, this is due to editing the file directly

@ghost
Copy link
Author

ghost commented Jul 4, 2022

@ignotus666 Yeah, That was my question #2683
OK, I shall do it right now. and Commit again soon.

@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Ok. Compiled and included all the necessary files in Jamulus.pro and the qrc file in src. Now there's an error (to me unreadable):

QString::arg: 1 argument(s) missing in 왼쪽 및 오른쪽 로컬 오디오 채널 사이의 레벨을 적절하게 제어합니다. 모노 신호의 경우 두 채널 사이의 팬 역할을 합니다. 예를 들어 마이크가 오른쪽 입력 채널에 연결되어 있고 악기가 마이크보다 훨씬 큰 왼쪽 입력 채널에 연결된 경우 페이더 위의 레이블이 %1을 표시하는 방향으로 오디오 페이더를 이동합니다. 여기서 % 2는 현재 감쇠 표시기입니다.

@ghost
Copy link
Author

ghost commented Jul 4, 2022

@ann0see That line is
message
location filename="../clientsettingsdlg.cpp" line="59"/
source
Controls the relative levels of the left and right local audio channels. For a mono signal it acts as a pan between the two channels. For example, if a microphone is connected to the right input channel and an instrument is connected to the left input channel which is much louder than the microphone, move the audio fader in a direction where the label above the fader shows %1, where %2 is the current attenuation indicator.
/source
translation
왼쪽 및 오른쪽 로컬 오디오 채널 사이의 레벨을 적절하게 제어합니다. 모노 신호의 경우 두 채널 사이의 팬 역할을 합니다. 예를 들어 마이크가 오른쪽 입력 채널에 연결되어 있고 악기가 마이크보다 훨씬 큰 왼쪽 입력 채널에 연결된 경우 페이더 위의 레이블이 %1을 표시하는 방향으로 오디오 페이더를 이동합니다. 여기서 % 2는 현재 감쇠 표시기입니다.
/translation
/message

@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Ok. Wait a moment, I've just added my additions to your branch

@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Ok. I think now I understand what the problem is: You added a wrong space at this line between % and 2

@ghost
Copy link
Author

ghost commented Jul 4, 2022

OK, I understand it.

@ann0see ann0see changed the title Create translation_ko_KR.ts Add Korean translation Jul 4, 2022
@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Ok. Just checked it on Debian and it compiles and seems to run. Nevertheless: In around 30 to 45 minutes GitHub should be finished and you should check the compiled app from the Checks tab. Some things don't seem to be translated yet, but that's ok

@ghost
Copy link
Author

ghost commented Jul 4, 2022

@ann0see
I think it should be "src/translation/translation_ko_Kr.qm" changed to "src/translation/translation_ko_KR.qm" in f4176b2

@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Eh. Yes. You should be able to change that too. (Only on my phone at the moment)

@ann0see
Copy link
Member

ann0see commented Jul 4, 2022

Ok. Can you please now check the app if everything looks ok?

@ghost
Copy link
Author

ghost commented Jul 5, 2022

It looks Good!
EEF67424-4E6B-4D90-8501-24FB2B81A66C

@ghost
Copy link
Author

ghost commented Jul 5, 2022

I'm awating other user and musicians feedback now.
If it is OK for them, Let me tell you.
but, for me? it's OK now, good to go.

@ghost
Copy link
Author

ghost commented Jul 5, 2022

Just edited by accepting feedback from Korean users and,
It is their (one of users) actual running image of Jamulus through Win 11
IMG_8648

@ghost
Copy link
Author

ghost commented Jul 5, 2022

Now, I edited all.
This is the end.
Thank you all of you :)

@ann0see
Copy link
Member

ann0see commented Jul 5, 2022

Great!

To merger: this should be squash merged.

@ghost
Copy link
Author

ghost commented Jul 6, 2022

@pljones Should I affect it here as well?
#2710
#2695

  • line 737 source and translation to "Now a directory"
  • line 648 source and translation to "Directory server list full"

@pljones
Copy link
Collaborator

pljones commented Jul 6, 2022

@pljones Should I affect it here as well?

Thanks, yes. Can you refresh your master from upstream, rebase and take the necessary steps, please.

@ghost
Copy link
Author

ghost commented Jul 6, 2022

yes sure,
It's done and Is it OK now?

* Translation: Add Korean translation to Jamulus.pro
@ann0see
Copy link
Member

ann0see commented Jul 6, 2022

I've now rebased and pushed it. Let's hope I pushed the correct version

@gilgongo
Copy link
Member

gilgongo commented Jul 6, 2022

Hi @bagjunggyu - thanks for contributing! We'd like to invite you to our Discord/Matrix channel where we coordinate releases and discuss details on things like translation.

If you want, please send an email to team@jamulus.io and request an invite. Thanks!

@ann0see
Copy link
Member

ann0see commented Jul 6, 2022

@gilgongo I already asked:

#2685 (comment)
However, we should still add him to the translators group on GitHub probably?

@ghost
Copy link
Author

ghost commented Jul 6, 2022

Thank you very much all of you.
Actually, I'm not a developer like you guys, until now to me, github was just a memo pad :)
I'm trying to rebase as following this manual

*git remote add upstream git@github.com:jamulussoftware/jamulus ;# add the main repo as upstream remote if you didn’t already as mentioned above
*git fetch upstream ; # Get the latest changes from the main upstream repo you added before (if needed)
*git checkout <theBranchYourWorkIsOn = "patch-1" in this case right?> ;# switch to (= checkout) the branch you want to rebase
*git rebase upstream/master ;# replay (=rebase) your changes onto the latest code (and fix conflicts if needed)
*git rebase --continue # to continue the rebasing progress after having fixed the conflicts
*git push --force # Push the changes to your repo. Be aware that this will overwrite your remote branch!

is it right?

@ghost
Copy link
Author

ghost commented Jul 6, 2022

@gilgongo Anyway, Thank you very much for your kind inviting me :)

Add Jung-Kyu Park(bagjunggyu) as Korean translator.
@pljones
Copy link
Collaborator

pljones commented Jul 6, 2022

git remote add upstream [git@github.com](mailto:git@github.com):jamulussoftware/jamulus ;# add the main repo as upstream remote if you didn’t already as mentioned above

As it notes, you do this only once. You then have "origin" as your github repo and "upstream" as the main Jamulus repo.

git fetch upstream ; # Get the latest changes from the main upstream repo you added before (if needed)

This should say git pull upstream master.

git checkout <theBranchYourWorkIsOn = "patch-1" in this case right?> ;# switch to (= checkout) the branch you want to rebase

Yes.

git rebase upstream/master ;# replay (=rebase) your changes onto the latest code (and fix conflicts if needed)

No - don't rebase onto upstream/master ever! It's a moving target. Always do git rebase -i master and check that the commits you see are the commits you've made and nothing else that you didn't do on your branch.

git rebase --continue # to continue the rebasing progress after having fixed the conflicts

Only if you got conflicts, of course.

Now, as an additional check, before the last step:

git diff master

and review your change to make sure it contains everything you want and nothing you do not want.

Finally:

git push --force # Push the changes to your repo. Be aware that this will overwrite your remote branch!

This tells Github you've changed your branch content. Normally --force isn't wanted, as "rewriting history" is bad... usually. But when doing a rebase, it's essential.

Jung-Kyu Park added 2 commits July 7, 2022 03:32
Add Jung-Kyu Park(bagjunggyu) as Korean translator.
src/util.cpp Outdated
@@ -729,14 +729,15 @@ CAboutDlg::CAboutDlg ( QWidget* parent ) : CBaseDlg ( parent )
"<p><b>" +
tr ( "Slovak" ) +
"</b></p>"
"<p>Jose Riha (<a href=\"https://github.com/jose1711\">jose1711</a>)</p>" +
"<p><b>" + tr ( "Simplified Chinese" ) +
"<p>Jose Riha (<a href=\"https://github.com/jose1711\">jose1711</a>)</p>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was almost correct. You need to add yourself without a "+" similar to the translators above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :) I would try again.

Add Jung-Kyu Park(bagjunggyu) as Korean translator.
@ghost
Copy link
Author

ghost commented Jul 6, 2022

@pljones
Should I do command "s" for squash after "git rebase -i master" ?

@ghost
Copy link
Author

ghost commented Jul 6, 2022

@ann0see It means already rebased? and nothing left to do?

@ann0see

This comment was marked as duplicate.

@ann0see
Copy link
Member

ann0see commented Jul 6, 2022

You still need to squash, yes ;-).

@ann0see
Copy link
Member

ann0see commented Jul 6, 2022

I think it's ok like this (we can squash merge it).

There's still some internal stuff we need to do (add to create translation issues) but otherwise: ready for final review

@ghost
Copy link
Author

ghost commented Jul 6, 2022

@ann0see
Thank you :)
(actually, until now I have failed "git push --force" several times.)

@pljones
Copy link
Collaborator

pljones commented Jul 6, 2022

Approving despite my utter lack of Korean! Thanks!

@ghost
Copy link
Author

ghost commented Jul 6, 2022

@pljones Thank you very much :)

@ann0see ann0see merged commit 852c7f5 into jamulussoftware:master Jul 6, 2022
@ann0see ann0see added the needs documentation PRs requiring documentation changes or additions label Jul 6, 2022
@ghost ghost deleted the patch-1 branch July 6, 2022 21:35
@ann0see ann0see removed the needs documentation PRs requiring documentation changes or additions label Jul 6, 2022
@pljones pljones mentioned this pull request Jul 23, 2022
75 tasks
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.

4 participants