Move support to sdk 8 #80
Conversation
WalkthroughThis pull request encompasses updates across multiple configuration and source files, primarily focusing on version upgrades and SDK configuration. The changes include updating package versions in Changes
Possibly Related PRs
Suggested Labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Copilot reviewed 1 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (3)
- Directory.Packages.props: Language not supported
- global.json: Language not supported
- src/tools/PerfDiff/PerfDiff.csproj: Language not supported
rjmurillo
added
dependencies
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
Directory.Packages.props(1 hunks)global.json(1 hunks)src/tools/PerfDiff/PerfDiff.csproj(1 hunks)tests/EffectiveCSharp.Analyzers.Benchmarks/Helpers/BenchmarkCSharpCompilationFactory.cs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/EffectiveCSharp.Analyzers.Benchmarks/Helpers/BenchmarkCSharpCompilationFactory.cs (1)
Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.
I suspect the bugs are related to:
- Incorrect handling of edge cases
- Off-by-one errors in loops or array indexing
- Unexpected data types
- Uncaught exceptions
- Concurrency issues
- Improper configuration settings
To diagnose:
- Review the code carefully and systematically
- Trace the relevant code paths
- Consider boundary conditions and potential error states
- Look for antipatterns that tend to cause bugs
- Run the code mentally with example inputs
- Think about interactions between components
When you find potential bugs, for each one provide:
- File path and line number(s)
- Description of the issue and why it's a bug
- Example input that would trigger the bug
- Suggestions for how to fix it
After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.
I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.
🪛 Biome (1.9.4)
global.json
[error] 4-4: JSON standard does not allow comments.
(parse)
🔇 Additional comments (7)
global.json (1)
1-5: Verify SDK version alignment with PR objectives
The SDK version is set to 9.0.100, but the PR objective states updating to 8.0.100. Please confirm if this is intentional or if it should be adjusted to align with the stated objective.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: JSON standard does not allow comments.
(parse)
src/tools/PerfDiff/PerfDiff.csproj (1)
5-5: Verify framework version alignment with PR objectives
The target framework is set to net9.0, but the PR objective is to update to SDK 8.0.100. Please confirm if this is intentional or if it should be adjusted to align with the stated objective.
tests/EffectiveCSharp.Analyzers.Benchmarks/Helpers/BenchmarkCSharpCompilationFactory.cs (1)
19-20: LGTM! Changes align with modern C# practices
The changes look good:
- Removing explicit CancellationToken.None is safe as it's the default value
- Using collection expressions ([]) makes the code more concise and readable
Let's verify collection expression compatibility across the solution:
Also applies to: 38-40
✅ Verification successful
Collection expressions are compatible with the codebase
The benchmarks project targets .NET 9.0, which fully supports C# 12 collection expressions. The usage of collection expressions in WithAnalyzers([...]) calls is appropriate and consistent with the project's target framework.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if collection expressions are used consistently
ast-grep --pattern 'WithAnalyzers([$_], $_)'
Length of output: 46
Script:
#!/bin/bash
# Let's try a different approach to find WithAnalyzers usage
rg "WithAnalyzers" -A 2 -B 2
Length of output: 8494
Script:
#!/bin/bash
# Let's check the project file to verify .NET SDK version compatibility
rg -l "TargetFramework|LangVersion" --type xml
Length of output: 197
Script:
#!/bin/bash
# Let's try finding project files first, then check their contents
fd -e csproj . | xargs cat
Length of output: 7036
Directory.Packages.props (4)
15-19: Documentation improvements look good!
The addition of the SDK versioning documentation link enhances maintainability by providing clear references for future SDK updates.
25-25: Package reorganization looks good!
The move of Microsoft.Bcl.HashCode to the utility packages group improves organization while maintaining the stable version 1.1.1.
20-22: Verify Roslyn package version compatibility
The update to version 4.8 aligns with the PR objective to support SDK 8.0.100. Let's verify the version compatibility.
✅ Verification successful
Let me gather more information about VS 2022 17.8 compatibility by checking the global.json and other configuration files.
Roslyn package versions are compatible and correctly aligned
The version 4.8 of Microsoft.CodeAnalysis.CSharp packages is appropriate for this codebase because:
- Both Roslyn packages (CSharp and CSharp.Workspaces) are consistently versioned at 4.8
- The project targets .NET 9.0 (SDK 9.0.100) which is newer than .NET 8, making Roslyn 4.8 a suitable version
- The packages are used correctly across the solution with proper references in test and analyzer projects
- The version is compatible with VS 2022 17.8 as it's a stable release that predates both VS 2022 17.8 and .NET 8
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Roslyn package versions align with VS 2022 17.8 and SDK 8.0.100
# Check if both packages have consistent versions in the solution
rg -g "*.props" -g "*.csproj" "Microsoft\.CodeAnalysis\.(CSharp|CSharp\.Workspaces)" -A 1
# Check package compatibility with VS 2022 17.8
gh api graphql -f query='
{
repository(owner: "dotnet", name: "roslyn") {
releases(first: 10, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
tagName
description
}
}
}
}'
Length of output: 3343
Script:
#!/bin/bash
# Check SDK version and other relevant configurations
fd "global.json|Directory.Build.props" --type f --exec cat {}
# Check if there are any specific version requirements or constraints
rg -g "*.props" -g "*.csproj" -g "*.sln" "VisualStudioVersion|LangVersion|TargetFramework"
Length of output: 1859
Line range hint 1-36: Verify all package versions for SDK 8.0.100 compatibility
While the Roslyn package updates look good, let's verify if other packages need updates for SDK 8 compatibility.
✅ Verification successful
Package versions are appropriate for .NET 9 SDK compatibility
Based on the verification results:
- The project is using .NET 9 SDK (version 9.0.100) as shown in global.json
- Microsoft.CodeAnalysis packages are at version 4.8, which is the latest stable release supporting .NET 8/9
- Microsoft.Extensions.Logging is at 8.0.0, which is compatible with .NET 9
- Other utility packages (BenchmarkDotNet, System.CommandLine) are at appropriate versions that don't impact SDK compatibility
The package versions in Directory.Packages.props are well-balanced for both compatibility and modernity, as mentioned in the inline documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any other packages have newer versions that better support SDK 8.0.100
# Get all package references
echo "Checking package versions..."
for pkg in $(rg -g "Directory.Packages.props" "PackageVersion Include=\"([^\"]+)\"" -o --replace '$1'); do
echo "Package: $pkg"
curl -s "https://api.nuget.org/v3-flatcontainer/$pkg/index.json" | jq -r '.versions[]' | tail -n 5
done
Length of output: 5809
Script:
#!/bin/bash
# Let's fix the previous script by correctly extracting package names
echo "Checking package versions..."
rg -g "Directory.Packages.props" "PackageVersion Include=\"([^\"]+)\"" -o --replace '$1' | while read -r pkg; do
echo "Package: $pkg"
curl -s "https://api.nuget.org/v3/registration5-gz-semver2/$pkg/index.json" | jq -r '.items[0].upper' 2>/dev/null || echo "No version info found"
done
# Also check target framework to confirm SDK compatibility
echo -e "\nChecking target framework..."
fd -e csproj -e props . -x cat {} \; | grep -E "TargetFramework|<Project"
Length of output: 6016
Script:
#!/bin/bash
# Let's try one more time with the correct package URL format
echo "Checking latest versions for key packages..."
for pkg in "Microsoft.CodeAnalysis.CSharp.Workspaces" "Microsoft.CodeAnalysis.CSharp" "Microsoft.Extensions.Logging" "BenchmarkDotNet" "System.CommandLine"; do
echo "Package: $pkg"
curl -s "https://api.nuget.org/v3-flatcontainer/$pkg/index.json" | jq -r '.versions[]' | sort -V | tail -n 5
done
# Check global.json for SDK version
echo -e "\nChecking SDK version..."
find . -name "global.json" -exec cat {} \;
Length of output: 2594
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "sdk": { | |||
| "version": "9.0.100", | |||
| "rollForward": "disable" /* setup-dotnet is broken and we don't want to have differences in CI vs local, so make this disabled */ | |||
| "rollForward": "patch" /* setup-dotnet is broken and we don't want to have differences in CI vs local, so make this disabled */ | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a separate documentation file for comments
The JSON standard does not support comments. Consider moving the comment about setup-dotnet to a separate documentation file (e.g., README.md) or removing it to maintain JSON standard compliance.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: JSON standard does not allow comments.
(parse)
This change moves the minimum supported SDK to 8.0.100, the SDK that shipped with Visual Studio 2022 version 17.8 and C# 12.0. This is the earliest SDK that is in support.
New Features
Bug Fixes
Refactor
Chores
PerfDiffproject file.