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

Fix nb_NO translation integration #108

Merged
merged 6 commits into from
Aug 15, 2021
Merged

Conversation

comradekingu
Copy link
Contributor

@comradekingu comradekingu commented Aug 13, 2021

Don't think this does anything, but I also can't figure out why the build breaks…

There is an extra / in https://github.com/falzonv/discreet-launcher/pull/77/files#diff-d0c61e533993db7b1d779138cff623d901b1893361374df34f72cc7d92c7a246R33
but that looks to have been removed when merging.

@comradekingu comradekingu changed the title Draft: Fix nb_NO translation integration Fix nb_NO translation integration Aug 13, 2021
@falzonv
Copy link
Owner

falzonv commented Aug 14, 2021

Yes, I had fixed the extra / in a later commit.

By the way, I see that we forgot to apply the commit for your mail address in #77 (link to the comment: https://github.com/falzonv/discreet-launcher/pull/77/files#r687027736).
I added it to this PR as a suggestion and let you see if you want to include it or not ;-)

Also added the two strings which were missing (thank you!), since this PR is there.

@falzonv
Copy link
Owner

falzonv commented Aug 14, 2021

In "strings.xml", there are three translatable="false" strings that I am not sure what to do with:

  • Since they are URLs and my name, they are indeed not translatable and it makes sense to add this label.
  • On the other hand, because of this label, Android Studio gives me a warning if they appear in other translations files than the default translation (which is why I removed them for "nb-rNO" and "ru" in 94e6bfa).
    It doesn't break the build but I don't like to let warnings remain for things that could be easily fixed.

Which approach do you think is better?
1/ Remove this "not translatable" label and have the three strings in all translations files.
2/ Keep the label like now and have them only in the main translation file (in that case we need to remove them from this PR).

@comradekingu
Copy link
Contributor Author

@falzonv Removed since they aren't found in the other translated resources.

@falzonv
Copy link
Owner

falzonv commented Aug 15, 2021

Perfect, let's merge then!

@falzonv falzonv merged commit 75a1812 into falzonv:main Aug 15, 2021
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