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

[PTRun][WindowWalker]Fix analyzer errors #16887

Merged

Conversation

jaimecbernardo
Copy link
Collaborator

Summary of the Pull Request

What is this about:
After merging #16555, #16325 added some analyzer errors, making so that main no longer builds successfully.

What is included in the PR:
Fixes for the analyzer errors.

How does someone test / validate:
CI is building again.

Quality Checklist

  • Linked issue: Enable analyzers for each project #16511
  • Communication: I've discussed this with core contributors in the issue.
  • Tests: Added/updated and all pass
  • Installer: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Docs: Added/ updated
  • Binaries: Any new files are added to WXS / YML

@htcfreek
Copy link
Collaborator

htcfreek commented Mar 8, 2022

@yuyoyuppe
I not sure which culture types are good to choose here. So I am interested in your opinion.

@jaimecbernardo
Copy link
Collaborator Author

@yuyoyuppe I not sure which culture types are good to choose here. So I am interested in your opinion.

I imagine this will only make a difference for desktop.ToString().ToUpper( for DesktopNames. Even so, I don't think it'll make much difference. Other uses are pretty much for english based characters.

@jaimecbernardo jaimecbernardo merged commit eb961ee into microsoft:main Mar 8, 2022
@CleanCodeDeveloper
Copy link
Contributor

I not sure which culture types are good to choose here. So I am interested in your opinion.

@htcfreek: You can have a look at the Microsoft recommendations for string usage

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.

4 participants