Skip to content
This repository was archived by the owner on Jul 6, 2026. It is now read-only.

[jnimarshalmethod-gen] Localizable errors - #696

Merged
jonpryor merged 10 commits into
dotnet:masterfrom
radekdoulik:pr-localizable-errors-and-warnings
Sep 4, 2020
Merged

[jnimarshalmethod-gen] Localizable errors#696
jonpryor merged 10 commits into
dotnet:masterfrom
radekdoulik:pr-localizable-errors-and-warnings

Conversation

@radekdoulik

@radekdoulikradekdoulik commented Aug 25, 2020•
edited
Loading

Copy link
Copy Markdown
Member

and warnings messages. Uses the same Java.Interop.Localization assembly
as other JI tools use for localizable text.

Change some of the warnings into informational output and also extend
some of the warnings with more information how to possibly avoid
the warning.

@radekdoulikradekdoulik changed the title Pr localizable errors and warnings[jnimarshalmethod-gen] Localizable errorsAug 25, 2020
jpobst
jpobst reviewed Aug 25, 2020
View reviewed changes
Comment threadtools/jnimarshalmethod-gen/App.cs Outdated
brendanzagaeski
brendanzagaeski reviewed Aug 26, 2020
View reviewed changes

@brendanzagaeskibrendanzagaeski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a bunch for setting all this up! It looks pretty good already. I'll leave a first round of comments with some questions and some ideas about wording adjustments and comments that might help the localization team. I'll be sure to take another look to make sure everything looks good after the messages get code numbers. Thanks!

Comment threadsrc/Java.Interop.Localization/Resources.resx
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadtools/jnimarshalmethod-gen/App.cs Outdated
Comment threadtools/jnimarshalmethod-gen/App.cs
Comment threadtools/jnimarshalmethod-gen/App.cs
Comment threadtools/jnimarshalmethod-gen/App.cs
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadtools/jnimarshalmethod-gen/TypeMover.cs
Comment threadtools/jnimarshalmethod-gen/TypeMover.cs Outdated
Comment threadtools/jnimarshalmethod-gen/TypeMover.cs Outdated
@radekdoulik
[jnimarshalmethod-gen] Localizable errors
02d3a54
and warnings messages. Uses the same Java.Interop.Localization assembly
as other JI tools for localizable messages.
@radekdoulik
Use the right string
1b1650a
@radekdoulik
Remove clutter
c5411a2
@radekdoulik
Print error and warning codes
f897210
Also use lower case for error/warning words to be better compatible with
our (tasks)[https://github.com/xamarin/xamarin-android/blob/f14890fa2146913afb023a37b6ff4ec62381392a/src/Xamarin.Android.Build.Tasks/Utilities/MSBuildExtensions.cs#L78-L79].
@radekdoulik
Add few comments
892dbc9
Also turn warning about missing symbols to informational output.
@radekdoulik
Additional information to some of the warnings
4edf9ca
Also change 2 warning to informational output
@radekdoulik
Update xlf files
715c6d7
@radekdoulik
Extended one of the comments
79b9edb
@radekdoulik
radekdoulikforce-pushed the pr-localizable-errors-and-warnings branch from 6ee069f to 79b9edbCompareAugust 28, 2020 11:59
brendanzagaeski
brendanzagaeski reviewed Aug 28, 2020
View reviewed changes

@brendanzagaeskibrendanzagaeski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Thanks for the changes and additional info on these messages! I'll add one more set of wording ideas to consider, and then we should be close to ready to wrap this up.

Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
brendanzagaeski
brendanzagaeski reviewed Sep 3, 2020
View reviewed changes
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
Comment threadsrc/Java.Interop.Localization/Resources.resx Outdated
brendanzagaeski
brendanzagaeski approved these changes Sep 4, 2020
View reviewed changes

@brendanzagaeskibrendanzagaeski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! Thanks again for all the work on this!

@jonpryor
jonpryor merged commit 8ea0bb2 into dotnet:masterSep 4, 2020
@jpobstjpobst added this to the 11.1 (16.9 / 8.9) milestone Sep 8, 2020
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 13, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jonpryorjonpryorAwaiting requested review from jonpryor
2 more reviewers
@jpobstjpobstjpobst left review comments
@brendanzagaeskibrendanzagaeskibrendanzagaeski approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

11.2 (16.9 / 8.9)

Development

Successfully merging this pull request may close these issues.

4 participants

@radekdoulik@jonpryor@jpobst@brendanzagaeski