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

Introduce resx for BuildValidator and MS.CA.Rebuild to allow localization #53447

Merged
merged 2 commits into from May 21, 2021

Conversation

Youssef1313
Copy link
Member

Fixes #52349

  • I added periods to the end of exception messages when moving to resx.

  • Enclosed variable inputs to the messages with single quotes.

  • Replaced an existing double quotes with single quotes to be consistent since this is what used across the whole codebase.

  • TODO: Haven't yet completed moving everything.

  • Are there any projects other than BuildValidator and Rebuild?

cc @RikkiGibson

@Youssef1313 Youssef1313 requested review from a team as code owners May 16, 2021 07:49
@Youssef1313 Youssef1313 changed the title Introduce resx for BuildValidator and MS.CA.Rebuild to allow localizatuib Introduce resx for BuildValidator and MS.CA.Rebuild to allow localization May 16, 2021
Copy link
Contributor

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

Are there any projects other than BuildValidator and Rebuild?

No, the only other thing we have related to this feature is the test project, which doesn't need to be localized.

Thanks so much for taking a stab at this. I think remaining stuff we would want to move includes:

  • Description strings in the CLI

    "--assembliesPath", "Path to assemblies to rebuild (can be specified one or more times)"

  • We have a lot of messages that we log at this point in the BuildValidator. Honestly not sure if we want them sticking around long-term, or whether we care about localizing things that would only show up in "verbose" mode, or what... so should probably punt on it.

Will try to take a closer look later to determine what else would need to be moved.

@@ -31,7 +31,7 @@ private static string GetIldasmPath()
}
else
{
throw new PlatformNotSupportedException("Runtime platform not supported for testing");
throw new PlatformNotSupportedException(BuildValidatorResources.Runtime_platform_not_supported_for_testing);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in this case, it's sufficiently clear if the message string is removed entirely.

@RikkiGibson
Copy link
Contributor

@jaredpar for second review

@RikkiGibson
Copy link
Contributor

Thanks for the contribution @Youssef1313! Will file a follow-up so we don't forget about doing something with the log messages.

@RikkiGibson RikkiGibson merged commit 17facfa into dotnet:main May 21, 2021
@ghost ghost added this to the Next milestone May 21, 2021
@Youssef1313 Youssef1313 deleted the localize branch May 22, 2021 05:42
333fred added a commit that referenced this pull request May 24, 2021
…ures/interpolated-string

* upstream/main: (92 commits)
  Keep casts when necessary to prefer a constant pattern over a type pattern
  Remove SyntaxKind.DataKeyword (#53614)
  Display 'readonly' for record structs (#53634)
  Update Building, Debugging, and Testing on Windows.md (#53543)
  Update dependencies from https://github.com/dotnet/arcade build 20210521.3 (#53617)
  Introduce resx for BuildValidator and MS.CA.Rebuild to allow localization (#53447)
  Report obsoletion diagnostics for slice and indexer (#53463)
  Update BasicGenerateConstructorDialog.cs
  Add searchbox in generate overrides dialog
  Allow `with` on anonymous types (#53248)
  Report diagnostic on correct node (#53538)
  Fix NotNullIfNotNull delegate conversion (#53409)
  Verify quick info session in InvokeQuickInfo
  Remove unnecessary retry
  Ensure no navbar IO on the UI thread
  Enable nullable reference types
  Fix timeout behavior in GetQuickInfo
  Add a semantic model based GetQuickInfoAsync entry point into QuickInfoServiceWithProviders
  Move semantic model based quick info API up to CommonQuickInfoProvider type
  Fix dnceng build by forcing the use of xcopy msbuild
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BuildValidator should use a resx to enable localization
3 participants