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

Remove unnecessary WithCulture() from localizers #17439

Merged
merged 3 commits into from
Feb 8, 2025
Merged

Conversation

hishamco
Copy link
Member

No description provided.

@@ -87,16 +87,6 @@
</ItemGroup>
</Target>

<Target Name="UpdateModuleAssets"
Copy link
Member

Choose a reason for hiding this comment

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

What was it doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Believe it or not, I don't know :), seems JT did is for a reason when there's WithCultures() in the past

Copy link
Member

Choose a reason for hiding this comment

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

Then perhaps better not to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's related to the WithCulture in the localization API

Copy link
Contributor

Choose a reason for hiding this comment

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

See https://learn.microsoft.com/en-us/visualstudio/msbuild/assignculture-task?view=vs-2022:

Files: Specifies the list of files with embedded culture names to assign a culture to. The task tries to figure out if each file is a culture-specific resource, and if so, what culture. To skip this detection process and force a file to be culture-neutral, set the metadata entry WithCulture to false.

Copy link
Member Author

@hishamco hishamco Feb 1, 2025

Choose a reason for hiding this comment

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

This change was introduced in #1483, TBH I have never seen such this while working on localization, so I will revert this change and let us move this PR forward

@hishamco hishamco requested a review from sebastienros February 1, 2025 16:59
@hishamco hishamco merged commit 52fd154 into main Feb 8, 2025
12 checks passed
@hishamco hishamco deleted the hishamco/with-culture branch February 8, 2025 00:07
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