⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@viceroypenguin
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings February 9, 2026 22:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an explicit configuration in the shared MSBuild props to restrict Meziantou.Polyfill usage to only the System.Threading.Lock polyfill across the solution.

Changes:

  • Introduces an “Enabled Polyfills” property group in Directory.Build.props.
  • Sets MeziantouPolyfill_IncludedPolyfills to target T:System.Threading.Lock.

Comment on lines +21 to 26
<Polyfill></Polyfill>
<Polyfill>$(Polyfill)|T:System.Threading.Lock</Polyfill>
<MeziantouPolyfill_IncludedPolyfills>$(Polyfill)</MeziantouPolyfill_IncludedPolyfills>
</PropertyGroup>

<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

<Polyfill></Polyfill> followed by <Polyfill>$(Polyfill)|T:System.Threading.Lock</Polyfill> expands to a value that starts with a leading | (because $(Polyfill) is empty). If MeziantouPolyfill_IncludedPolyfills doesn’t tolerate empty entries, this can prevent the Lock polyfill from being applied (and break net8 builds). Prefer setting MeziantouPolyfill_IncludedPolyfills directly to T:System.Threading.Lock, or append conditionally only when the existing value is non-empty (and avoid the extra Polyfill property altogether).

Suggested change
<Polyfill></Polyfill>
<Polyfill>$(Polyfill)|T:System.Threading.Lock</Polyfill>
<MeziantouPolyfill_IncludedPolyfills>$(Polyfill)</MeziantouPolyfill_IncludedPolyfills>
</PropertyGroup>
<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
<MeziantouPolyfill_IncludedPolyfills>T:System.Threading.Lock</MeziantouPolyfill_IncludedPolyfills>
</PropertyGroup>
<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">

Copilot uses AI. Check for mistakes.
</PropertyGroup>

<PropertyGroup Label="Enabled Polyfills">
<!--https://github.com/meziantou/Meziantou.Polyfill?tab=readme-ov-file#supported-polyfills-->
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

XML comment formatting: add a space after <!-- for readability/consistency (e.g., <!-- https://... -->).

Suggested change
<!--https://github.com/meziantou/Meziantou.Polyfill?tab=readme-ov-file#supported-polyfills-->
<!-- https://github.com/meziantou/Meziantou.Polyfill?tab=readme-ov-file#supported-polyfills-->

Copilot uses AI. Check for mistakes.
@viceroypenguin viceroypenguin merged commit 2806ce5 into master Feb 9, 2026
7 checks passed
@viceroypenguin viceroypenguin deleted the fix branch February 9, 2026 23:02
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.

1 participant