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

bring riseupvpn experiments to ooni android #393

Merged
merged 14 commits into from
Jan 12, 2021

Conversation

cyBerta
Copy link
Contributor

@cyBerta cyBerta commented Nov 19, 2020

@cyBerta cyBerta marked this pull request as draft November 19, 2020 20:20
@lorenzoPrimi
Copy link
Contributor

Thanks for this PR @cyBerta, I will follow it closely!
Please let me know when the strings are definitive so I can upload them to transifex to let the translators work on them

@cyBerta
Copy link
Contributor Author

cyBerta commented Dec 7, 2020

In strings.xml the link to the test description is provided. There's a pending PR ooni/ooni.org#716 which will add the page to the ooni website.

@cyBerta cyBerta changed the title [WIP] bring riseupvpn experiments to ooni android bring riseupvpn experiments to ooni android Dec 8, 2020
@cyBerta cyBerta marked this pull request as ready for review December 8, 2020 12:53
<string name="TestResults_Details_Circumvention_Riseupvpn_Table_Header_Bridge">Bridged connections</string>
<string name="TestResults_Overview_Circumvention_Riseupvpn_Api_Blocked">blocked</string>
<plurals name="TestResults_Overview_Circumvention_Riseupvpn_Blocked">
<item quantity="one">%d blocked</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

NOTE TO SELF:
remove these quantity and add

  • TestResults_Overview_Circumvention_Riseupvpn_Blocked_Singular
  • TestResults_Overview_Circumvention_Riseupvpn_Blocked_Plural

Copy link
Contributor Author

@cyBerta cyBerta Jan 11, 2021

Choose a reason for hiding this comment

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

@lorenzoPrimi I think it would be better to generally use quantity strings, because there are languages (like Polish or Russian) that have more than 1 plural form. The translation would be grammatically wrong in these cases.

Copy link
Contributor Author

@cyBerta cyBerta Jan 11, 2021

Choose a reason for hiding this comment

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

@lorenzoPrimi this snippet illustrates it nicely with the Russian example of pineapples (ананас).
https://gist.github.com/Eeyore741/a0228dd9b7551daaf1fe8fc11a3548fc#file-plurals_en_ru-csv

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately this would bring our translation system based on transifex, it's a much bigger change that can't be handled in this PR.

@@ -133,6 +135,10 @@
<item quantity="one">@string/TestResults_Summary_Websites_Hero_Reachable_Singular</item>
<item quantity="other">@string/TestResults_Summary_Websites_Hero_Reachable_Plural</item>
</plurals>
<plurals name="TestResults_Overview_Circumvention_RiseupVPN_Blocked" translatable="false">
<item quantity="one">@string/TestResults_Overview_Circumvention_RiseupVPN_Blocked_Singular</item>
<item quantity="other">@string/TestResults_Overview_Circumvention_RiseupVPN_Blocked_Plural</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

@cyBerta we still use the plurals here.
If you want to help us improve our translation system, let me know we can discuss it in a github issue.
Very appreciated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if it's not super urgent to be finished, I can definitely work on that. +1 for leaving this issue aside here and for opening an extra ticket for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm gonna move ahead, merge this issue and opened another ticket about the plurals: ooni/translations#16
I still don't understand why our approach (string.xml for strings and untransatable.xml for plurals) it can cause that problem as we still use the quantities.

@lorenzoPrimi lorenzoPrimi merged commit 50e49cf into ooni:master Jan 12, 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