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

started localization for the app and added first german local #447

Merged
merged 17 commits into from
Jun 9, 2023

Conversation

kuro-codes
Copy link
Contributor

I started the effort to localize the app. For whatever reason i don't understand the app wanted to have a DB version bump.
I tested it on emulator and a device.
There are a whole lot of duplicate words in the first step but i thought it would be easier to keep a naming scheme that is easy to understand then to remove duplicates and make it more complicated for everyone in the first place.
Getting rid of duplicates would be another step in this feature i think.

@kuro-codes kuro-codes requested a review from dessalines as a code owner June 7, 2023 12:27
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

This is a great first step! I've internationalized a few other android apps, but was really daunted by this one. Laying the groundwork like you've done here is the difficult part :), thank you!

Run ./gradlew formatKotlin to pass lint, then once I get a minute I'll also grep through the code to make sure nothing's missing.

Once this passes

@@ -302,8 +302,15 @@ val MIGRATION_9_10 = object : Migration(9, 10) {
}
}

val MIGRATION_10_11 = object : Migration(10, 11) {
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here? You should probably merge from main.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've had the issue that after an update the app crashed and told me i need a migration to work. Tried it that way. Maybe its not correct. Sorry i am new to this 😓
Will try to revert this.

Copy link
Member

Choose a reason for hiding this comment

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

Haha no problem. Just merge from main and you should be good.

@kuro-codes
Copy link
Contributor Author

I ran formatKotlin. Please be so kind and check if updating from the former version is running. :) For now a few strings in the Toasts are missing because of the missing context in those cases but i think its a start.

@dessalines
Copy link
Member

Hrm... formatting still had issues. https://woodpecker.join-lemmy.org/dessalines/jerboa/pipeline/128/3

@kuro-codes
Copy link
Contributor Author

Wasn't the formatting, it was a merge error, i fixed it right now.

@dessalines
Copy link
Member

For now a few strings in the Toasts are missing because of the missing context in those cases but i think its a start.

Could you add context to those?

It'd be good to get all the strings now, because once people start adding languages, it'll be tougher later on.

If not I can probably help out in a day or two.

@kuro-codes
Copy link
Contributor Author

And ran into another merge conflict, you guys are working to fast 😂

@kuro-codes
Copy link
Contributor Author

For now a few strings in the Toasts are missing because of the missing context in those cases but i think its a start.

Could you add context to those?

It'd be good to get all the strings now, because once people start adding languages, it'll be tougher later on.

If not I can probably help out in a day or two.

Honestly i can look into it tomorrow. Its 11:30pm here and i had a few Whiskeys already. I am well beyond the Ballmer Peak 😅

@kuro-codes
Copy link
Contributor Author

Give me a minute, i've found the context solution. Maybe the Ballmer Peak is not a lie...

@kuro-codes
Copy link
Contributor Author

I have added the three toasts i have found and it should work now. Please check it if you can.

@dessalines
Copy link
Member

Mmmk I'll check now.

@dessalines
Copy link
Member

I found a good number of toasts and Text that still need in Utils, CommentNode, Login....

Do a Find in files in android studio, then search for Text(

That should give you a full list.

@twizmwazin twizmwazin linked an issue Jun 7, 2023 that may be closed by this pull request
@kuro-codes
Copy link
Contributor Author

Searched for Text( and text = to be sure to find everything. I think i have it covered now.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Do the search again, I found a lot that were still missing:

CommentNode L419, CommentNode L620 , etc.

@kuro-codes kuro-codes requested a review from twizmwazin as a code owner June 8, 2023 19:43
@kuro-codes
Copy link
Contributor Author

I did again a search for text =, then for (text = which resulted in other entries, then i opted to search via regex for /.+([A-z0-9])/. I hope i have found everything.

@kuro-codes
Copy link
Contributor Author

kuro-codes commented Jun 9, 2023

I don't see why the build is broken. Local build runs fine. Is it possible to restart the build?

Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)

@twizmwazin twizmwazin mentioned this pull request Jun 9, 2023
@MV-GH
Copy link
Collaborator

MV-GH commented Jun 9, 2023

Seems the Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed) is due to (lack of) memory issues. A potential solution I got from here is to add this to the woodpecker CI config

environment:
    -  GRADLE_OPTS=-Dorg.gradle.jvmargs="-XX:MaxMetaspaceSize=1g"

Not sure how effective this would be though.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Okay I think that's all of em, tested and it works great!

Thank you so much for doing this!

I'll handle the merge conflicts.

@dessalines dessalines merged commit 05b5771 into LemmyNet:main Jun 9, 2023
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.

Add i18n / internationized strings
4 participants