Upgrade project to .net10 #496
Conversation
WalkthroughProjects and CI updated to target .NET 10.0: project TargetFrameworks changed from Changes
Sequence Diagram(s)(Skipped — changes are build/config updates and do not modify runtime control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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 |
github-actions
bot
added
api
Tides
added
epic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Obsidian.Tests/Obsidian.Tests.csproj (1)
19-29: System.Drawing.Common on net10.0: verify OS/runtime usage.Still pinned to 9.0.4 and Windows-only at runtime; tests on Linux/macOS can fail if types are exercised. Either gate such tests to Windows or consider removing the dependency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Obsidian.API/Obsidian.API.csproj(2 hunks)Obsidian.ConsoleApp/Obsidian.ConsoleApp.csproj(2 hunks)Obsidian.Nbt/Obsidian.Nbt.csproj(1 hunks)Obsidian.Tests/Obsidian.Tests.csproj(1 hunks)Obsidian/Obsidian.csproj(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: .NET Build
Obsidian.API/Obsidian.API.csproj
[error] 1-1: dotnet restore failed: NETSDK1045 - The current .NET SDK does not support targeting .NET 10.0. Target .NET 9.0 or lower, or use a newer .NET SDK that supports .NET 10.0.
🔇 Additional comments (8)
Obsidian.Tests/Obsidian.Tests.csproj (1)
4-4: TFM upgrade looks good; ensure CI has .NET 10 SDK.Change is correct. Given the repo-wide move to net10.0, make sure the CI/runtime SDK is updated, otherwise restores/builds will fail.
Obsidian.Nbt/Obsidian.Nbt.csproj (1)
4-4: Straightforward TFM bump.No issues spotted.
Obsidian.API/Obsidian.API.csproj (2)
4-4: TFM bump is fine.Aligned with the rest of the solution.
78-83: RC Microsoft.Extensions abstractions: verify compatibility plan.Using 10.0.0-rc.1 is reasonable during migration; please confirm downstream consumers/plugins won’t bind to 9.x at runtime. Plan to move to 10.0.0 RTM once available.
Obsidian/Obsidian.csproj (2)
5-5: TFM bump acknowledged.No objections.
53-56: RC Microsoft.Extensions + System.IO.Hashing: check cross-package alignment and runtime binding.Ensure all app entry points (ConsoleApp, tests) and any plugin host match 10.0.0-rc.1 to avoid assembly load/binding mismatches with 9.x libraries. Track an issue to flip to 10.0.0 RTM.
Also applies to: 64-64
Obsidian.ConsoleApp/Obsidian.ConsoleApp.csproj (2)
5-5: TFM upgrade looks good.
17-17: Hosting RC package: confirm coherence with library/API versions.Version matches the library’s RC line; validate runtime coherence across all Microsoft.Extensions.* packages.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/nuget-push.yml (2)
35-42: Use resolvable SDK specifiers (10.0.x, 9.0.x, …) instead of bare majors.setup-dotnet expects semver-like ranges. Using just “10” can fail to resolve on runners. Align with dotnet.yml and pin minor streams.
Apply this diff:
- - name: Setup .NET - uses: actions/setup-dotnet@v5 - with: - dotnet-version: | - 10 - 9 - 8 - 7 + - name: Setup .NET + uses: actions/setup-dotnet@v5 + with: + dotnet-version: | + 10.0.x + 9.0.x + 8.0.x + 7.0.x
5-16: Tag trigger is gated by paths filter (likely prevents tag publishes).push.tags and push.paths are ANDed. Semver tag pushes won’t run unless those files changed in the tag commit, which defeats “Publish semver tags as releases.”
Two viable fixes—pick one:
- Option A: Let tags always trigger (remove paths filter):
push: branches: ["1.21.x"] # Publish semver tags as releases. tags: ["v*.*.*"] - # only publish when we change a file here - paths: - - "Obsidian.API/**/*.cs" - - "Obsidian.API/*.csproj" - - "Obsidian/Assets/**/*.json" - - "Obsidian.SourceGenerators/**/*.cs" - - "Obsidian.SourceGenerators/*.csproj" - - ".github/workflows/nuget-push.yml"
- Option B: Keep paths gating and drop tag trigger (use workflow_dispatch to publish tags):
push: branches: ["1.21.x"] - # Publish semver tags as releases. - tags: ["v*.*.*"] # only publish when we change a file here paths: - "Obsidian.API/**/*.cs" - "Obsidian.API/*.csproj" - "Obsidian/Assets/**/*.json" - "Obsidian.SourceGenerators/**/*.cs" - "Obsidian.SourceGenerators/*.csproj" - ".github/workflows/nuget-push.yml"
🧹 Nitpick comments (6)
.github/workflows/nuget-push.yml (2)
43-47: Avoid rebuilding during pack to speed up pushes.You build then pack (which rebuilds). Skip the second build.
Apply this diff:
- dotnet pack -c Release -o build -p:Version=1.0.0-nightly-${{ github.run_number }} + dotnet pack -c Release --no-build -o build -p:Version=1.0.0-nightly-${{ github.run_number }}
35-42: Enable NuGet caching to speed restores.setup-dotnet v5 supports built‑in caching.
Apply this diff:
- name: Setup .NET uses: actions/setup-dotnet@v5 with: dotnet-version: | 10.0.x 9.0.x 8.0.x 7.0.x + cache: true + cache-dependency-path: | + **/*.csproj + **/global.json.github/workflows/dotnet.yml (4)
27-30: actions/setup-dotnet@v5 with 10.0.x: LGTM. Consider adding cache.Add built‑in NuGet cache to cut CI time.
Apply this diff:
- name: Setup .NET uses: actions/setup-dotnet@v5 with: - dotnet-version: 10.0.x + dotnet-version: 10.0.x + cache: true + cache-dependency-path: | + **/*.csproj + **/global.json
35-40: Avoid absolute runner paths; use github.workspace for portability.Absolute paths are brittle across runners and org settings.
Apply this diff:
- - name: Rename executables - working-directory: /home/runner/work/Obsidian/Obsidian/Obsidian.ConsoleApp/bin/Release/net10.0/ + - name: Rename executables + working-directory: ${{ github.workspace }}/Obsidian.ConsoleApp/bin/Release/net10.0/ run: | mv Obsidian.ConsoleApp.dll ObsidianApp.dll mv Obsidian.ConsoleApp.deps.json ObsidianApp.deps.json mv Obsidian.ConsoleApp.runtimeconfig.json ObsidianApp.runtimeconfig.json
41-45: Use workspace-relative artifact path.Same portability rationale as above.
Apply this diff:
- name: Upload a Build Artifact uses: actions/upload-artifact@v4 with: name: Obsidian-Nightly-Build - path: /home/runner/work/Obsidian/Obsidian/Obsidian.ConsoleApp/bin/Release/net10.0/ + path: ${{ github.workspace }}/Obsidian.ConsoleApp/bin/Release/net10.0/
22-33: Optional hardening: pin actions by commit SHA.Locks supply chain; keep the @v5/@v4 tags as comments for readability.
Example:
- - uses: actions/checkout@v5 + - uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v5 - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@1d771c2f74f74cd610a9eb77b0395297dbf1a7e3 # v5 - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/dotnet.yml(2 hunks).github/workflows/nuget-push.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/nuget-push.yml (1)
30-33: Upgrade to actions/checkout@v5 looks good.No issues spotted with the checkout step change.
.github/workflows/dotnet.yml (1)
23-23: actions/checkout@v5: LGTM.
Summary by CodeRabbit