Skip to content

Enable the Respecting Item's Culture by default#41042

Merged
marcpopMSFT merged 5 commits intodotnet:mainfrom
f-alizada:dev/f-alizada/enable-respect-culture-by-default
Jun 11, 2024
Merged

Enable the Respecting Item's Culture by default#41042
marcpopMSFT merged 5 commits intodotnet:mainfrom
f-alizada:dev/f-alizada/enable-respect-culture-by-default

Conversation

@f-alizada
Copy link
Copy Markdown
Contributor

@f-alizada f-alizada commented May 21, 2024

MSBuild task: AssignCulture was recently updated and new parameter of the task was added:
RespectAlreadyAssignedItemCulture property. dotnet/msbuild#10026

Related issues:
dotnet/msbuild#1454
dotnet/msbuild#9954

@f-alizada f-alizada requested a review from rainersigwald May 21, 2024 09:20
@ghost ghost added Area-Infrastructure untriaged Request triage from a team member labels May 21, 2024
@f-alizada f-alizada requested a review from baronfel May 21, 2024 15:02
<GenerateResourceWarnOnBinaryFormatterUse>true</GenerateResourceWarnOnBinaryFormatterUse>
</PropertyGroup>

<PropertyGroup Condition="'$(RespectAlreadyAssignedItemCulture)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '9.0'))">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rainersigwald we should be using MSBuild changewave checks here, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I prefer that to a TF check like this, would we really want to change behavior on this based on SDK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh duh, now the conversation comes flooding back to me. Yes I agree now, and we can disregard my earlier question.

Copy link
Copy Markdown
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Looks good. I'm not sure we need an automated test for this since it looks like it's covered in MSBuild, but it would be good to do manual sanity testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Infrastructure untriaged Request triage from a team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants