Skip to content

Conversation

@dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Sep 16, 2020

Fixed #72

@dellis1972 dellis1972 marked this pull request as ready for review September 23, 2020 20:21
Copy link

@brendanzagaeski brendanzagaeski left a comment

Choose a reason for hiding this comment

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

Quick first note and recap of some discussion outside GitHub:

It looks like the messages here are mostly exception messages. Since this is the first xamarin-android dependency to tackle exception messages, that raises some small questions about what kind of adjustments to make to the wording of the exception messages to help make them more easily localizable. Based on a first glance at the current wordings, I think maybe using full sentences where it's not too awkward might be worthwhile.

To give a tiny first example, maybe must not be null or empty can be adjusted just slightly to add the name of the argument at the beginning of the message. That will align it with some similar messages in dotnet/sdk and make it a full sentence:

 <data name="MustNotBeNullOrEmpty" xml:space="preserve">
-  <value>Must not be null or empty.</value>
-  <comment>Must not be null or empty.</comment>
+  <value>{0} must not be null or empty.</value>
+  <comment>{0} - The name of the parameter</comment>
 </data>

Though this is technically a little redundant since the exception also accepts the type name as a separate argument and surfaces it separately, the benefit is that the redundancy provides more context for the localization team.

Copy link

@brendanzagaeski brendanzagaeski left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this! Here are a few comments and questions about the localization build setup. I'll plan to do one more pass to look through the .resx file and the message wordings tomorrow.

Copy link

@brendanzagaeski brendanzagaeski left a comment

Choose a reason for hiding this comment

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

Nice work on the new message wordings! Here are ideas for a few little adjustments. One other little bit of tidy up that might be a nice finishing touch could be to alphabetize the entries by name in the .resx file.

@dellis1972 dellis1972 merged commit 3fa042b into dotnet:master Jan 6, 2021
@dellis1972 dellis1972 deleted the localise branch January 6, 2021 13:38
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.

Localise all strings within LibZipSharp.

3 participants