-
-
Notifications
You must be signed in to change notification settings - Fork 0
Modernize repository to match other ImmediatePlatform products #84
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
Conversation
There was a problem hiding this 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; setsLangVersionto 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> | |||
|
|
|||
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
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.
| <ItemGroup Condition="'$(TargetFramework)' == 'net8.0'"> | |
| <Using Include="System.Object" Alias="Lock" /> | |
| </ItemGroup> |
| <PropertyGroup> | ||
| <TargetFrameworks>net8.0;net9.0</TargetFrameworks> | ||
| <OutputType>Exe</OutputType> | ||
| </PropertyGroup> | ||
|
|
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
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).
| <PropertyGroup> | ||
| <LangVersion>latest</LangVersion> | ||
| <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks> | ||
| <LangVersion>14</LangVersion> |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
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.
| <LangVersion>14</LangVersion> | |
| <LangVersion>latest</LangVersion> |
No description provided.