⚠ 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 21:03
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

Modernizes the repo’s build/test/release setup to align with other ImmediatePlatform products by centralizing framework configuration, updating dependencies, switching the functional tests over to xUnit v3 (MTP), and updating GitHub Actions workflows (including NuGet publishing via OIDC).

Changes:

  • Centralize multi-targeting and compiler settings in Directory.Build.props (now targets net8.0/net9.0/net10.0; sets LangVersion to 14).
  • Update package versions and test dependencies (move functional tests to xunit.v3.mtp-v2, update analyzers/polyfill).
  • Update CI/release workflows (add .NET 10, adjust dotnet test, and use NuGet OIDC login for publishing).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/Immediate.Cache.FunctionalTests/Immediate.Cache.FunctionalTests.csproj Updates test project dependencies and properties (now relies on centralized TFM settings).
tests/Immediate.Cache.FunctionalTests/ApplicationCacheTests.cs Migrates test attributes to xUnit and threads cancellation tokens through calls.
src/Immediate.Cache/Immediate.Cache.csproj Removes per-project TFMs to inherit from centralized build props.
src/Immediate.Cache.Shared/Immediate.Cache.Shared.csproj Removes per-project TFMs and simplifies project file (now inherits centralized settings).
Directory.Packages.props Updates central package versions; adds net10.0-conditioned package versions; updates analyzer/polyfill setup.
Directory.Build.props Centralizes TargetFrameworks and sets LangVersion to 14; moves warnings-as-errors to CI only.
.github/workflows/release.yml Updates release pipeline; adds .NET 10; switches NuGet publishing to OIDC-based login.
.github/workflows/build.yml Updates CI build/test to include .NET 10 and removes GitHubActions logger usage.

@@ -12,8 +11,4 @@
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
</ItemGroup>

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.

Removing the conditional MSBuild <Using Alias="Lock" Include="System.Object" ...> breaks builds for net8.0: the code uses the Lock type (e.g., ApplicationCacheBase.cs), but Lock is only available on newer frameworks. Since the repo still targets net8.0 (via Directory.Build.props), reintroduce a conditional alias for net8.0 (or change the code to use object / conditional compilation) so multi-targeting continues to compile.

Suggested change
<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<Using Include="System.Object" Alias="Lock" />
</ItemGroup>

Copilot uses AI. Check for mistakes.
Comment on lines 3 to 6
<PropertyGroup>
<TargetFrameworks>net8.0;net9.0</TargetFrameworks>
<OutputType>Exe</OutputType>
</PropertyGroup>

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.

This test project still targets net8.0 (via Directory.Build.props) but no longer defines the conditional Lock alias; DelayGetValue.cs uses Lock, which will fail to compile on net8.0. Add back a conditional MSBuild <Using Alias="Lock" Include="System.Object" ...> for net8.0 (or update the code to avoid Lock on older TFMs).

Copilot uses AI. Check for mistakes.
<PropertyGroup>
<LangVersion>latest</LangVersion>
<TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks>
<LangVersion>14</LangVersion>
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.

Setting <LangVersion>14</LangVersion> effectively requires a .NET SDK/new enough compiler for all contributors. Since there is no global.json in the repo to pin/declare the required SDK, builds may fail inconsistently depending on the locally installed SDK. Consider adding a global.json that pins a compatible SDK (or reverting to latest/latestMajor) to make the requirement explicit.

Suggested change
<LangVersion>14</LangVersion>
<LangVersion>latest</LangVersion>

Copilot uses AI. Check for mistakes.
@viceroypenguin viceroypenguin merged commit 5b0ef94 into master Feb 9, 2026
1 check passed
@viceroypenguin viceroypenguin deleted the modernize branch February 9, 2026 21:17
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