-
Notifications
You must be signed in to change notification settings - Fork 112
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
U.S. English translation revamp and cleanup of other lang files #736
U.S. English translation revamp and cleanup of other lang files #736
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR. I really appreciate you taking the time to make everything more consistent.
I have a few issues with the descriptions. Mostly periods at the end of sentences and the removal of the If
in the configs.
I generally prefer to only capitalize text in titles, not buttons, etc.
Please don't take my comments as offensive. I really appreciate the work :)
common/src/main/java/de/maxhenkel/voicechat/config/ServerConfig.java
Outdated
Show resolved
Hide resolved
I've answered all your questions and addressed any concerns you might have had, I hope. If there's a question with no answer, it means that the answer has already been given elsewhere. (If it is not and I have missed something, please let me know.) Have a nice day! |
393a36d
to
434ccb7
Compare
I reviewed and answered all your comments :) |
We are getting closer and closer to the final result that both of us can be satisfied with, I hope. I'm sorry that it's taking so long and that the progress since last time is minimal, but I'm only working on it in my spare time, making just a few changes each day. |
434ccb7
to
eca085e
Compare
No worries :) |
I've finished and published everything I had planned for this review cycle earlier today. If you find some time, you can check it out. |
Will do thanks! |
Alright, I think i've answered everything |
Thank you for your patience. The good news is that there are only a few topics left that need your input. |
Thanks, I think ive answered everything. |
I have a few last review questions for you. Please take a look at them when you find some time. |
Done |
d7d19c1
to
2ae5739
Compare
This pull request is now ready for a final review or two. It’s been a long journey, and I really appreciate your help and time! |
Just ignore it. I'll do a final pass when I have time and merge it if I don't find anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except fore some very minor things and something I noticed needs changing in general.
common/src/main/java/de/maxhenkel/voicechat/config/ClientConfig.java
Outdated
Show resolved
Hide resolved
common/src/main/java/de/maxhenkel/voicechat/config/ClientConfig.java
Outdated
Show resolved
Hide resolved
common/src/main/java/de/maxhenkel/voicechat/config/ServerConfig.java
Outdated
Show resolved
Hide resolved
270b652
to
330f38d
Compare
common-proxy/src/main/java/de/maxhenkel/voicechat/config/ProxyConfig.java
Outdated
Show resolved
Hide resolved
I found 1 small thing again that I forgot to tell you about, sorry. |
330f38d
to
e6d480a
Compare
Thanks a lot! |
If you have any suggestions or small things that you notice, you can add me on Discord, my name is |
I personally don't use Discord, but thanks for the invitation :) |
First, a quick note that this pull request is technically a draft and will initially be labeled as such here on GitHub, so every single change can be freely discussed without any time pressure.
To simplify the review process, this pull request consists of three main parts (one commit for each part):
cloth_config.voicechat.category.other
translation key has been renamed tocloth_config.voicechat.category.ingame_menu
to better reflect the new value it now conveys.message.voicechat.enabled
andmessage.voicechat.disabled
have been renamed tomessage.voicechat.denoiser.on
andmessage.voicechat.denoiser.off
respectively to show the logical connection between them and the corresponding configuration option they define (i.e.message.voicechat.denoiser
). Additionally, this also replaces the long descriptive labels such as "enabled" and "disabled" with their shorter and generic equivalents: "ON" and "OFF".Please review these changes carefully, as the description above only summarizes the most significant ones, not all of them. I'm open to all kinds of criticism and comments because I realize that what seems like a positive change to me, may actually be the opposite. Furthermore, this is your mod, your repo, your rules (not mine), and I'm just trying to help as much as I can.