V9.0.0/rtm #102
V9.0.0/rtm #102
Conversation
|
Warning Rate limit exceeded@gimlichael has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 7 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. WalkthroughThe pull request includes updates to documentation files reflecting the support for .NET 9 across various components, replacing mentions of .NET 6. Additionally, it removes specific entries from the "Other Projects" section in the table of contents. A new CI/CD pipeline is introduced, and existing workflow files are modified to enhance project selection logic. Package versions are updated to stable releases, and a Docker image version is updated in the testing environment configuration. Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
.github/workflows/delayed-pipelines.yml (3)
22-63: Consider optimizing preparation jobs using matrix strategyThe Linux and Windows preparation jobs are identical except for the runner. Consider consolidating them using a matrix strategy to reduce code duplication.
- prepare_linux: - name: 🐧 Prepare Linux - runs-on: ubuntu-22.04 - timeout-minutes: 15 + prepare: + name: 🔧 Prepare ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-22.04, windows-2022] + include: + - os: ubuntu-22.04 + emoji: 🐧 + - os: windows-2022 + emoji: 🪟 + runs-on: ${{ matrix.os }} + timeout-minutes: 15
180-188: Add health check for SQL Server containerConsider adding a health check to ensure SQL Server is ready before running tests:
uses: codebeltnet/docker-compose@v1 with: command: up - options: --wait + options: --wait --health-cmd="sqlcmd -U sa -P $SA_PASSWORD -Q 'SELECT 1'" --health-interval=10s --health-timeout=5s --health-retries=3
296-307: Consider improving configuration handlingThe configuration fallback logic could be moved to a reusable expression:
- configuration: ${{ inputs.configuration == '' && 'Release' || inputs.configuration }} + configuration: ${{ coalesce(inputs.configuration, 'Release') }}.github/workflows/pipelines.yml (1)
136-179: Consider improving maintainability of project selection.The current approach of listing all projects in the script might become harder to maintain as the project grows. Consider these alternatives:
- Use a configuration file to store project lists
- Utilize glob patterns to reduce repetition
Example approach using a configuration file:
# .github/project-matrix.yml frameworks: modern: # net9.0, net8.0 patterns: - "src/**/Cuemon.AspNetCore*.csproj" - "src/**/Cuemon.Core.csproj" # ... other patternsThen modify the workflow to:
- projects=( - "src/**/Cuemon.AspNetCore.csproj" - # ... long list of projects - ) + # Read from configuration file + projects=$(yq e '.frameworks.modern.patterns[]' .github/project-matrix.yml)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
.docfx/includes/availability-all.md(1 hunks).docfx/includes/availability-default.md(1 hunks).docfx/includes/availability-hybrid.md(1 hunks).docfx/includes/availability-modern.md(1 hunks).docfx/toc.yml(0 hunks).github/workflows/delayed-pipelines.yml(1 hunks).github/workflows/pipelines.yml(1 hunks)Directory.Packages.props(3 hunks)testenvironments.json(1 hunks)
💤 Files with no reviewable changes (1)
- .docfx/toc.yml
✅ Files skipped from review due to trivial changes (5)
- .docfx/includes/availability-all.md
- .docfx/includes/availability-default.md
- .docfx/includes/availability-hybrid.md
- .docfx/includes/availability-modern.md
- testenvironments.json
🧰 Additional context used
🪛 actionlint
.github/workflows/delayed-pipelines.yml
100-100: shellcheck reported issue in this script: SC2086:info:2:81: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:7:56: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:13:56: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/delayed-pipelines.yml
[error] 79-79: trailing spaces
(trailing-spaces)
🔇 Additional comments (8)
Directory.Packages.props (3)
52-57: LGTM! Consistent version updates for .NET 9.
The package versions are consistently updated to stable 9.0.0 releases across all dependencies in the .NET 9 target framework section. This maintains version parity and aligns well with the .NET 9 support objective.
31-31: Verify compatibility with .NET Standard 2.x applications.
The updates to Microsoft.Bcl.AsyncInterfaces (9.0.0) and System.Threading.Tasks.Extensions (4.6.0) in the .NET Standard 2.x section should maintain backward compatibility. Please ensure these versions work correctly with existing .NET Standard 2.x applications.
#!/bin/bash
# Description: Check package compatibility with .NET Standard 2.x
# Search for .NET Standard project files and their target framework versions
fd -e csproj | xargs grep -l "netstandard2" | while read -r file; do
echo "Checking $file..."
cat "$file"
doneAlso applies to: 40-40
8-11: Consider using stable versions instead of release candidates.
The Codebelt.Extensions packages are currently set to release candidate versions (9.0.0-rc.1). While this aligns with the .NET 9 support objective, consider using stable versions if they become available to ensure production reliability.
✅ Verification successful
Error encountered in previous script. Running a corrected version to verify stable package versions.
Continue using release candidate versions for .NET 9 support.
The latest stable versions of the Codebelt.Extensions packages are 8.4.1 and 8.4.0. Using release candidate versions (9.0.0-rc.1) is necessary to support .NET 9. Monitor for stable releases to ensure production reliability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if stable versions are available for the Codebelt packages
# Check NuGet for latest versions of each package
for package in "Codebelt.Extensions.Xunit" "Codebelt.Extensions.Xunit.Hosting" "Codebelt.Extensions.Xunit.Hosting.AspNetCore" "Codebelt.Extensions.YamlDotNet"; do
echo "Checking $package..."
curl -s "https://api.nuget.org/v3-flatcontainer/$package/index.json" | jq -r '.versions[]' | grep -v '-' | tail -n 1
done
Length of output: 5913
Script:
#!/bin/bash
# Description: Check if stable versions are available for the Codebelt packages
# List of Codebelt packages to check
packages=(
"Codebelt.Extensions.Xunit"
"Codebelt.Extensions.Xunit.Hosting"
"Codebelt.Extensions.Xunit.Hosting.AspNetCore"
"Codebelt.Extensions.YamlDotNet"
)
# Iterate over each package and fetch the latest stable version
for package in "${packages[@]}"; do
echo "Checking $package..."
latest=$(curl -s "https://api.nuget.org/v3-flatcontainer/${package,,}/index.json" | jq -r '.versions[]' | grep -v '-' | tail -n 1)
if [ -n "$latest" ]; then
echo "Latest stable version of $package is $latest"
else
echo "No stable versions found for $package"
fi
done
Length of output: 1988
.github/workflows/delayed-pipelines.yml (4)
1-20: LGTM! Well-structured workflow triggers
The workflow triggers and manual dispatch configuration are properly defined with appropriate path exclusions.
92-98: Verify secure handling of signing key
The SNK file download uses secure secrets, but consider:
- Adding error handling for the download step
- Cleaning up the sensitive file after use
132-156: LGTM! Well-structured packaging job
The packaging job is properly configured with appropriate dependencies and matrix strategy.
207-295: LGTM! Comprehensive code analysis setup
The analysis jobs (SonarCloud, CodeCov, and CodeQL) are well configured with proper security measures and dependencies.
.github/workflows/pipelines.yml (1)
136-179: LGTM! Comprehensive project selection for modern .NET frameworks.
The project selection logic appropriately includes all necessary components, particularly the ASP.NET Core related projects for modern .NET frameworks (net9.0 and net8.0). The grouping is logical and ensures complete coverage of the codebase.
| - name: Set environment variable for projects | ||
| run: | | ||
| if [ "${{ matrix.framework }}" == "netstandard2.1" ]; then | ||
| echo "PROJECTS=src/**/Cuemon.Extensions.IO.csproj src/**/Cuemon.IO.csproj" >> $GITHUB_ENV | ||
| elif [ "${{ matrix.framework }}" == "netstandard2.0" ]; then | ||
| projects=( | ||
| "src/**/Cuemon.Extensions.Globalization.csproj" | ||
| ) | ||
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | ||
| else | ||
| projects=( | ||
| "src/**/Cuemon.Core.App.csproj" | ||
| "src/**/Cuemon.Extensions.Globalization.csproj" | ||
| ) | ||
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | ||
| fi | ||
| shell: bash |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve project selection logic and fix shell script issues
The current implementation has several areas for improvement:
- Shell script is vulnerable to word splitting (flagged by shellcheck)
- Project selection logic could be more maintainable using a configuration file
- echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV
+ echo "PROJECTS=${projects[*]@Q}" >> $GITHUB_ENVConsider moving the project selection logic to a configuration file:
# .github/build-matrix.yml
frameworks:
netstandard2.1:
- src/**/Cuemon.Extensions.IO.csproj
- src/**/Cuemon.IO.csproj
netstandard2.0:
- src/**/Cuemon.Extensions.Globalization.csproj
default:
- src/**/Cuemon.Core.App.csproj
- src/**/Cuemon.Extensions.Globalization.csproj🧰 Tools
🪛 actionlint
100-100: shellcheck reported issue in this script: SC2086:info:2:81: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:7:56: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:13:56: Double quote to prevent globbing and word splitting
(shellcheck)
| - name: Install .NET | ||
| uses: codebeltnet/install-dotnet@v1 | ||
| with: | ||
| includePreview: true |
There was a problem hiding this comment.
💡 Codebase verification
Remove unnecessary includePreview: true flag from workflow
No projects target preview .NET versions. Consider removing the includePreview: true setting to streamline the pipeline.
.github/workflows/delayed-pipelines.yml:35
🔗 Analysis chain
Review the inclusion of preview .NET versions in production pipeline
The includePreview: true flag is set for .NET installation. Consider if preview versions are necessary for your production pipeline, as they might introduce instability.
Also applies to: 56-56
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any projects specifically require preview .NET versions
rg -l "TargetFramework.*preview" "**/*.csproj"
Length of output: 100
Script:
#!/bin/bash
# List all .csproj files in the repository
fd -e csproj
Length of output: 5820
Script:
#!/bin/bash
# Search for TargetFramework entries with 'preview' in .csproj files
fd -e csproj --exec rg "TargetFramework.*preview"
Length of output: 51
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/delayed-pipelines.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/delayed-pipelines.yml
75-75: property "test-projects" is not defined in object type {dotnet-build: {conclusion: string; outcome: string; outputs: {string => string}}; minver-calculate: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
100-100: shellcheck reported issue in this script: SC2086:info:5:56: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:11:56: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/delayed-pipelines.yml
[error] 79-79: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/delayed-pipelines.yml (4)
1-20: LGTM! Well-structured workflow triggers
The workflow triggers are properly configured with:
- Comprehensive paths-ignore list for non-code changes
- Well-defined manual trigger with configuration choice
22-42: LGTM! Well-structured Linux preparation job
The Linux preparation job is properly configured with:
- Appropriate timeout settings
- Proper caching strategy for dependencies
- Clear output variables
125-151: LGTM! Well-structured pack job
The pack job configuration is solid with:
- Clear matrix strategy
- Proper artifact handling
- Appropriate job dependencies
152-164: LGTM! Well-structured deploy job
The deploy job is properly configured with:
- Appropriate PR condition
- Production environment protection
- Clear version referencing
| outputs: | ||
| version: ${{ steps.minver-calculate.outputs.version }} | ||
| testProjects: ${{ steps.test-projects.outputs.result }} |
There was a problem hiding this comment.
Fix missing test-projects output
The test-projects output is referenced but never set in the workflow.
Either:
- Remove the unused output if not needed
- Add a step to set this output if it's required:
- id: test-projects
name: Set Test Projects
run: echo "result=src/**/*.Tests.csproj" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint
75-75: property "test-projects" is not defined in object type {dotnet-build: {conclusion: string; outcome: string; outputs: {string => string}}; minver-calculate: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
| - name: Set environment variable for projects | ||
| run: | | ||
| if [ "${{ matrix.framework }}" == "netstandard2.0" ]; then | ||
| projects=( | ||
| "src/**/Cuemon.Extensions.Globalization.csproj" | ||
| ) | ||
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | ||
| else | ||
| projects=( | ||
| "src/**/Cuemon.Core.App.csproj" | ||
| "src/**/Cuemon.Extensions.Globalization.csproj" | ||
| ) | ||
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | ||
| fi | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
Fix shell script issues to prevent word splitting
The current shell script is vulnerable to word splitting issues.
Apply this diff to fix the issues:
if [ "${{ matrix.framework }}" == "netstandard2.0" ]; then
projects=(
"src/**/Cuemon.Extensions.Globalization.csproj"
)
- echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV
+ echo "PROJECTS=${projects[*]@Q}" >> $GITHUB_ENV
else
projects=(
"src/**/Cuemon.Core.App.csproj"
"src/**/Cuemon.Extensions.Globalization.csproj"
)
- echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV
+ echo "PROJECTS=${projects[*]@Q}" >> $GITHUB_ENV
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Set environment variable for projects | |
| run: | | |
| if [ "${{ matrix.framework }}" == "netstandard2.0" ]; then | |
| projects=( | |
| "src/**/Cuemon.Extensions.Globalization.csproj" | |
| ) | |
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | |
| else | |
| projects=( | |
| "src/**/Cuemon.Core.App.csproj" | |
| "src/**/Cuemon.Extensions.Globalization.csproj" | |
| ) | |
| echo "PROJECTS=$(IFS=' '; echo "${projects[*]}")" >> $GITHUB_ENV | |
| fi | |
| shell: bash | |
| - name: Set environment variable for projects | |
| run: | | |
| if [ "${{ matrix.framework }}" == "netstandard2.0" ]; then | |
| projects=( | |
| "src/**/Cuemon.Extensions.Globalization.csproj" | |
| ) | |
| echo "PROJECTS=${projects[*]@Q}" >> $GITHUB_ENV | |
| else | |
| projects=( | |
| "src/**/Cuemon.Core.App.csproj" | |
| "src/**/Cuemon.Extensions.Globalization.csproj" | |
| ) | |
| echo "PROJECTS=${projects[*]@Q}" >> $GITHUB_ENV | |
| fi | |
| shell: bash |
🧰 Tools
🪛 actionlint
100-100: shellcheck reported issue in this script: SC2086:info:5:56: Double quote to prevent globbing and word splitting
(shellcheck)
100-100: shellcheck reported issue in this script: SC2086:info:11:56: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/delayed-pipelines.yml (2)
107-107: Optimize build artifact download patternThe pattern
'DelayedBuild.*.${{ matrix.configuration }}'might download more artifacts than needed since it matches any framework. This could slow down the workflow unnecessarily.Consider using a more specific pattern that matches only the required frameworks:
-downloadBuildArtifactPattern: 'DelayedBuild.*.${{ matrix.configuration }}' +downloadBuildArtifactPattern: 'DelayedBuild.{net9.0,net8.0,netstandard2.0}.${{ matrix.configuration }}'
121-122: Simplify duplicated configuration logicThe configuration fallback logic is duplicated across lines. This could lead to maintenance issues if the default value needs to change.
Consider setting the configuration once at the job level:
steps: - name: Set configuration run: echo "CONFIG=${{ inputs.configuration == '' && 'Release' || inputs.configuration }}" >> $GITHUB_ENV - uses: codebeltnet/nuget-push@v1 with: token: ${{ secrets.NUGET_TOKEN }} configuration: ${{ env.CONFIG }} downloadBuildArtifactName: NuGet-Delayed-${{ env.CONFIG }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/delayed-pipelines.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/delayed-pipelines.yml
56-56: shellcheck reported issue in this script: SC2086:info:5:56: Double quote to prevent globbing and word splitting
(shellcheck)
56-56: shellcheck reported issue in this script: SC2086:info:11:56: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/delayed-pipelines.yml
[error] 35-35: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/delayed-pipelines.yml (2)
27-29: Verify framework compatibility in matrix strategy
The matrix includes both .NET (net9.0, net8.0) and .NET Standard (netstandard2.0) frameworks. Ensure that the projects specified in the environment variables are compatible with all these frameworks.
#!/bin/bash
# Check framework compatibility in project files
for proj in $(fd -e csproj); do
echo "=== $proj ==="
rg "<TargetFramework" "$proj" -A 1
done4-9: Review paths-ignore configuration
The .github/** path in paths-ignore would prevent this workflow from running when changes are made to workflow files themselves. This could lead to workflow changes not being tested in pull requests.
Consider removing .github/** from paths-ignore or narrowing it down to specific subdirectories that don't include workflows.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
=======================================
Coverage 80.02% 80.02%
=======================================
Files 598 598
Lines 18502 18502
Branches 1886 1886
=======================================
Hits 14806 14806
Misses 3628 3628
Partials 68 68 ☔ View full report in Codecov by Sentry. |
|
This pull request includes various updates to documentation, workflows, package versions, and test environments. The most significant changes involve updating the availability documentation to include .NET 9, adding a new delayed CI/CD pipeline, and updating package versions.
Documentation Updates:
.docfx/includes/availability-all.md,.docfx/includes/availability-default.md,.docfx/includes/availability-hybrid.md, and.docfx/includes/availability-modern.mdto include .NET 9. [1] [2] [3] [4]Workflow Enhancements:
.github/workflows/delayed-pipelines.yml, which includes steps for preparing environments, building, testing, packaging, and deploying..github/workflows/pipelines.ymlto refine the project selection for different build configurations. [1] [2]Package Version Updates:
Directory.Packages.propsto include newer versions of several packages, includingBackport.System.Threading.Lock,Codebelt.Extensions.Xunit, andSystem.Data.SqlClient. [1] [2] [3]Test Environment Updates:
testenvironments.jsonto use a new Docker image for the Ubuntu test environment.Documentation Removal:
.docfx/toc.yml.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Package Updates
Configuration Changes