-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: support .NET standard 2.0 #20
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
…erences for compatibility
… consistency across workflows and examples
…age for .NET 7 and below
|
Warning Rate limit exceeded@arika0093 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughCI workflows updated to use .NET 10 and run a multi-version test matrix; example projects and some csproj targets bumped to net10.0; several projects now multi-target netstandard2.0/net6/net8/net10 (with conditional net48 on Windows); EF Core integration tests and related model classes removed; README updated to require C# 12+ (PolySharp fallback). Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Setup as actions/setup-dotnet
participant Runner as Job Runner
Dev->>GH: Push commits (workflows, csproj, tests, README)
note right of GH `#E6F0FF`: release workflows now use DOTNET_VERSION=10.0.x
GH->>Setup: setup-dotnet (dotnet-version: "10.0.x")
Setup-->>GH: dotnet 10 installed
GH->>Runner: run release jobs
note right of GH `#FFF4E6`: test workflow uses dotnet-versions: 6.0,8.0,10.0 and matrix.os
GH->>GH: expand matrix (os × dotnet-version)
loop per matrix entry
GH->>Setup: setup-dotnet (dotnet-version: matrix dotnet-version)
Setup-->>Runner: dotnet installed
Runner->>Runner: build & test on matrix.os
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
examples/Linqraft.Benchmark/Linqraft.Benchmark.csproj (1)
9-10: Consider pinning versions for consistent benchmark results.Using wildcard versions (
*and10.*) in a benchmark project can lead to inconsistent results when different versions are restored. Benchmark results should be reproducible.Consider pinning to specific versions:
- <PackageReference Include="BenchmarkDotNet" Version="*" /> - <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="10.*" /> + <PackageReference Include="BenchmarkDotNet" Version="0.14.0" /> + <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="10.0.0" />examples/Linqraft.Sample/Linqraft.Sample.csproj (1)
10-11: Consider the implications of wildcard versions in sample projects.Using wildcard versions (
10.*) means users cloning this repository at different times may get different dependency versions, potentially leading to inconsistent behavior or build failures if breaking changes are introduced in minor versions.For sample/example projects that users reference, consider using more specific version constraints or exact versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/release-github.yaml(1 hunks).github/workflows/release.yaml(1 hunks).github/workflows/test.yaml(1 hunks)README.ja.md(1 hunks)README.md(1 hunks)examples/Linqraft.Benchmark/Linqraft.Benchmark.csproj(1 hunks)examples/Linqraft.Sample/Linqraft.Sample.csproj(1 hunks)src/Directory.Build.props(1 hunks)src/Linqraft.SourceGenerator/Linqraft.SourceGenerator.csproj(0 hunks)src/Linqraft/Linqraft.csproj(0 hunks)src/Linqraft/packages.lock.json(2 hunks)tests/Directory.Build.props(2 hunks)tests/Linqraft.Tests/Linqraft.Tests.csproj(0 hunks)tests/Linqraft.Tests/UseEFCoreSenarioTest.cs(0 hunks)
💤 Files with no reviewable changes (4)
- tests/Linqraft.Tests/Linqraft.Tests.csproj
- src/Linqraft.SourceGenerator/Linqraft.SourceGenerator.csproj
- src/Linqraft/Linqraft.csproj
- tests/Linqraft.Tests/UseEFCoreSenarioTest.cs
🔇 Additional comments (12)
src/Linqraft/packages.lock.json (1)
1-125: Proceeding with verification of remaining compatibility concerns:Lock file correctly reflects .NETStandard 2.0 targeting with appropriate polyfill dependencies and no compatibility or security concerns.
The transition from net8.0 to .NETStandard,Version=v2.0 is properly reflected. System.Runtime.CompilerServices.Unsafe 6.0.0 can be used when targeting .NET 6 or earlier, confirming forward compatibility with the transitive 4.5.3 requirement from System.Memory. The added System.* polyfill packages (Memory, Buffers, Reflection.Metadata, Text.Encoding.CodePages, etc.) at pinned versions are standard backports known to be compatible with .NETStandard 2.0 and enable C# 12 language features on older frameworks via PolySharp.
Security verification confirmed no public CVEs or Microsoft advisories for System.Reflection.Metadata 8.0.0 or System.Text.Encoding.CodePages 7.0.0. Nerdbank.GitVersioning 3.9.50 contains no supported framework assets and has no dependencies, confirming compatibility as a build-time tool across all target frameworks.
.github/workflows/release-github.yaml (1)
17-17: Verify .NET 10.0.x availability and stability for GitHub releases.Same concern as in
.github/workflows/release.yaml- ensure .NET 10.0.x is stable before using it in release workflows.src/Directory.Build.props (1)
3-3: LGTM! Good choice for broad compatibility.Targeting netstandard2.0 provides excellent compatibility with older .NET versions (.NET Framework 4.6.1+, .NET Core 2.0+, and all modern .NET versions) while still allowing modern C# features through the LangVersion setting and PolySharp.
examples/Linqraft.Benchmark/Linqraft.Benchmark.csproj (1)
4-4: Verify .NET 10.0 compatibility.Ensure .NET 10.0 is released and stable before targeting it in example projects.
.github/workflows/test.yaml (1)
12-15: Good approach for multi-framework testing.Setting up multiple .NET versions (6.0.x, 8.0.x, 10.0.x) aligns well with the multi-targeting strategy in the test projects. This ensures compatibility across LTS and latest versions.
However, verify that .NET 10.0.x is available and stable.
README.ja.md (2)
211-211: LGTM! Helpful installation guidance.Adding the concrete installation command with the
--prereleaseflag provides clear guidance for users.
193-205: Documentation is accurate; no changes needed.The correct MSBuild LangVersion syntax uses the minor version (e.g., "12.0", "11.0", "10.0"), which matches the documentation. The C# 12 requirement explanation, PolySharp workaround, and configuration example are all correct.
tests/Directory.Build.props (2)
3-6: LGTM! Comprehensive multi-targeting strategy.Testing across net6.0, net8.0, net10.0, and net48 (Windows) provides excellent coverage of supported frameworks. This aligns well with the multi-version setup in the test workflow.
21-24: LGTM! PolySharp enables C# 12 features on older frameworks.Adding PolySharp correctly enables modern C# features on older target frameworks, which is essential for the C# 12 interceptors used by this library.
examples/Linqraft.Sample/Linqraft.Sample.csproj (1)
3-3: Verify .NET 10.0 compatibility.Ensure .NET 10.0 is released and stable before targeting it in sample projects that users may reference.
.github/workflows/release.yaml (1)
7-7: No issues found—.NET 10.0.x is production-ready..NET 10.0 is released (GA) and is an LTS release, making it suitable for production releases. The workflow change is appropriate.
README.md (1)
191-203: Documentation updates align well with .NET Standard 2.0 shift.The prerequisites section has been appropriately updated to reflect the new C# 12 language requirement (instead of .NET 8.0+ runtime requirement) and includes clear guidance for enabling C# 12 features on .NET 7 and below using PolySharp. The XML configuration example is correct and provides users with the exact setup needed.
…n Directory.Build.props for consistency
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)src/Linqraft/Linqraft.csproj(1 hunks)tests/Directory.Build.props(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🔇 Additional comments (3)
tests/Directory.Build.props (3)
3-6: LGTM! Multi-targeting configuration is correct.The multi-targeting setup appropriately tests across modern .NET versions (net6.0, net8.0, net10.0) and conditionally includes net48 on Windows, which is correct since .NET Framework is Windows-only.
22-25: PolySharp configuration looks appropriate.The PolySharp package is correctly configured with
PrivateAssets="all"to prevent transitive exposure and the asset inclusion settings are comprehensive for enabling C# 12 features on older target frameworks.
14-14: Reconsider: Downgrading to version16.*does not break .NET 10 compatibility.Microsoft.NET.Test.Sdk 17.14.1 targets .NET Standard 2.0, which is compatible with .NET 10.0. Since .NET 10 implements .NET Standard 2.0 and earlier, packages targeting .NET Standard 2.0 will work on .NET 10.0 projects. Version 16.* will function correctly with net10.0 targets.
However, while this downgrade won't break compatibility, consider that using significantly older versions may miss recent bug fixes and improvements. Evaluate whether you need the specific benefits of version 16.*, or if a more recent stable version would be preferable.
Likely an incorrect or invalid review comment.
…d package dependencies for .NET versions
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Directory.Build.props (1)
3-3: Consider using singularTargetFrameworkfor consistency.While
TargetFrameworks(plural) works with a single value, the standard convention is to useTargetFramework(singular) when targeting only one framework andTargetFrameworks(plural) for multiple targets.Apply this diff for consistency with .NET conventions:
- <TargetFrameworks>netstandard2.0</TargetFrameworks> + <TargetFramework>netstandard2.0</TargetFramework>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Directory.Build.props(1 hunks)src/Linqraft.SourceGenerator/Linqraft.SourceGenerator.csproj(1 hunks)src/Linqraft/Linqraft.csproj(1 hunks)src/Linqraft/packages.lock.json(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Linqraft.SourceGenerator/Linqraft.SourceGenerator.csproj
🧰 Additional context used
🪛 GitHub Actions: .NET Build and Test
src/Linqraft/Linqraft.csproj
[error] 1-1: NU1004: The project target frameworks are different than the lock file's target frameworks. Lock file target frameworks: netstandard2.0,net6.0,net8.0,net10.0. Project target frameworks net48,netstandard2.0,net10.0,net6.0,net8.0. The packages lock file is inconsistent with the project dependencies so restore can't be run in locked mode. Disable the RestoreLockedMode MSBuild property or pass an explicit --force-evaluate option to run restore to update the lock file.
[error] 1-1: Process completed with exit code 1.
🔇 Additional comments (2)
src/Linqraft/Linqraft.csproj (1)
3-6: Multi-targeting implementation looks correct.The multi-targeting configuration properly targets .NET Standard 2.0, .NET 6.0, 8.0, and 10.0, with conditional .NET Framework 4.8 support on Windows. This aligns well with the PR objectives to broaden compatibility.
src/Linqraft/packages.lock.json (1)
1-350: Lock file will be regenerated to fix the pipeline failure.This auto-generated lock file is currently out of sync with the project file (as indicated by the
NU1004pipeline error). Once you rundotnet restore --force-evaluateonsrc/Linqraft/Linqraft.csproj, this file will be automatically regenerated with the correct framework entries and ordering.No manual changes to this file are necessary—just commit the regenerated version after running the restore command.
This pull request updates the project to support .NET 10 and improves compatibility with older .NET versions by targeting .NET Standard 2.0 for the main library. It also updates documentation to clarify prerequisites and adds PolySharp for enabling newer C# features on older frameworks. Test and example projects are updated to use .NET 10, and unnecessary test code and dependencies are removed.
Platform and Framework Updates:
Linqraft) to target.NETStandard2.0instead of.NET 8.0, increasing compatibility with older .NET versions. [1] [2] [3] [4]release.yaml,release-github.yaml,test.yaml) and example projects to use.NET 10.0(and in tests, also.NET 6.0and.NET 8.0). [1] [2] [3] [4] [5] [6]Documentation and Compatibility:
READMEfiles to clarify that C# 12 is required, and added instructions for using PolySharp to enable C# 12 features on .NET 7 and below, including an example project file snippet. [1] [2]Testing Improvements:
net6.0,net8.0,net10.0, andnet48on Windows), and added PolySharp as a test dependency to enable C# 12 features on older frameworks. [1] [2]UseEFCoreSenarioTestfile, which contained in-memory EF Core scenario tests. [1] [2]Dependency Management:
packages.lock.jsonto reflect new dependencies and compatibility with.NETStandard2.0, including transitive dependencies required for .NET Standard support. [1] [2]Summary by CodeRabbit
Documentation
Chores
Tests