Update VTEX.API.csproj #374
Conversation
WalkthroughThe changes in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
github-actions
bot
added
the
size/XS
gstraccini
bot
added
the
☑️ auto-merge
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Src/VTEX.API/VTEX.API.csproj (1)
6-8: Enhance package metadata for better discoverabilityWhile the additions are good, there are some improvements that could be made:
Consider these enhancements:
- <Description>The VTEX platform (PaaS) (e-commerce) SDK. A .NET client to consume VTEX APIs for both Core and Framework projects</Description> + <Description>.NET SDK for VTEX e-commerce platform APIs, supporting both Core and Framework projects</Description> - <PackageTags>vtex paas client sdk ecommerce e-commerce platform apis</PackageTags> + <PackageTags>vtex;paas;sdk;ecommerce;api-client;dotnet</PackageTags> - <PackageReleaseNotes>Chore</PackageReleaseNotes> + <PackageReleaseNotes>Updated project structure and target framework configuration</PackageReleaseNotes>Changes explained:
- More concise description while maintaining clarity
- Semicolon-separated tags (NuGet convention)
- More descriptive release notes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Src/VTEX.API/VTEX.API.csproj(1 hunks)
🔇 Additional comments (3)
Src/VTEX.API/VTEX.API.csproj (3)
4-4: LGTM: Valid ProjectGuid addition
The ProjectGuid follows the correct format and is a good practice for project identification in Visual Studio.
Line range hint 1-15: Verify impact of LangVersion removal
The removal of the LangVersion property might affect which C# language features are available to the project.
#!/bin/bash
# Description: Check for language version settings in other configuration files
# and verify the impact of its removal
echo "Checking for language version settings in other files..."
rg -l "LangVersion" --type-add 'props:*.props' --type-add 'csproj:*.csproj' -t props -t csproj
echo "Checking C# language features usage..."
rg -l "record\s+class|record\s+struct|init;|required\s+\w+|switch\s+expression" --type csharp5-5: Verify $(DotNetVersions) variable definition
The change to use $(DotNetVersions) variable is good for maintainability, but we need to ensure it's properly defined.
github-actions
bot
added
size/S
github-actions
bot
added
size/M
github-actions
bot
added
size/L
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
1 similar comment
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
7 similar comments
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
.github/workflows/contributors-readme.yml (2)
15-17: Fix indentation in permissions blockThe explicit permissions declaration is a good security practice, following the principle of least privilege. However, the indentation needs to be adjusted to match the workflow structure.
Apply this diff to fix the indentation:
contrib-readme-job: runs-on: ubuntu-latest name: Update files - permissions: - contents: write - pull-requests: write + permissions: + contents: write + pull-requests: write🧰 Tools
🪛 yamllint
[warning] 16-16: wrong indentation: expected 12 but found 10
(indentation)
19-19: Remove trailing spacesThere are trailing spaces on this blank line that should be removed.
Apply this diff to fix the trailing spaces:
- +🧰 Tools
🪛 yamllint
[error] 19-19: trailing spaces
(trailing-spaces)
.github/workflows/codacy.yml (3)
16-23: Consider optimizing workflow triggers and scheduleThe current configuration might lead to redundant scans:
- Running on both
pushandpull_requesttomaincould result in duplicate scans for PRs that are merged.- The schedule time (17:42 UTC on Fridays) seems arbitrary. Consider scheduling during off-peak hours to optimize CI resources.
on: push: branches: [ "main" ] - pull_request: - # The branches below must be a subset of the branches above - branches: [ "main" ] schedule: - - cron: '42 17 * * 5' + - cron: '0 2 * * 1' # Run at 2 AM UTC on Mondays
25-27: Remove redundant top-level permissionThe
contents: readpermission is already defined at the job level with proper documentation. The top-level permission declaration is redundant.-permissions: - contents: read -
49-50: Consider parameterizing the SARIF file pathThe SARIF file path is hardcoded in multiple places. Consider using a workflow variable for better maintainability.
+env: + SARIF_FILE: results.sarif + jobs: codacy-security-scan: steps: - name: Run Codacy Analysis CLI with: - output: results.sarif + output: ${{ env.SARIF_FILE }} format: sarif - name: Upload SARIF results file with: - sarif_file: results.sarif + sarif_file: ${{ env.SARIF_FILE }}Also applies to: 60-61
Tests/VTEX.Tests/Health/VtexHealthClientTests.cs (1)
61-67: Consider adding test cases for error scenarios.While the current test covers both healthy and unhealthy statuses, it might be worth adding test cases for:
- API exceptions
- Timeout scenarios
- Empty or null responses
Would you like me to provide examples of these additional test cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
.gitattributes(1 hunks).github/dependabot.yml(2 hunks).github/workflows/codacy.yml(1 hunks).github/workflows/contributors-readme.yml(1 hunks).github/workflows/deep-source.yml(1 hunks).github/workflows/infisical-secrets-check.yml(1 hunks).github/workflows/publish-docs.yml(1 hunks).sonarlint/VTEX-SDK-dotnet.json(1 hunks).vscode/settings.json(1 hunks)SECURITY.md(0 hunks)Tests/VTEX.Tests/Health/VtexHealthClientTests.cs(3 hunks)Tests/VTEX.Tests/VTEX.Tests.csproj(1 hunks)
💤 Files with no reviewable changes (1)
- SECURITY.md
✅ Files skipped from review due to trivial changes (4)
- .gitattributes
- .github/workflows/infisical-secrets-check.yml
- .github/workflows/publish-docs.yml
- .sonarlint/VTEX-SDK-dotnet.json
🧰 Additional context used
🪛 yamllint
.github/workflows/contributors-readme.yml
[warning] 16-16: wrong indentation: expected 12 but found 10
(indentation)
[error] 19-19: trailing spaces
(trailing-spaces)
🪛 Biome
.vscode/settings.json
[error] 22-22: expected , but instead found "sonarlint.connectedMode.project"
Remove "sonarlint.connectedMode.project"
(parse)
🔇 Additional comments (10)
.github/dependabot.yml (2)
35-35: LGTM!
Simple formatting improvement for consistency.
19-22: LGTM! Verify CrispyWaffle package dependencies.
The addition of a CrispyWaffle group is a good practice to consolidate related package updates.
Let's verify the CrispyWaffle package dependencies in the project:
✅ Verification successful
CrispyWaffle package dependencies verified.
The grouping effectively consolidates updates for CrispyWaffle packages used in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for CrispyWaffle package references in the project
# Expected: Find .csproj files containing CrispyWaffle package references to confirm the grouping is useful
# Search for CrispyWaffle package references in .csproj files
fd -e csproj --exec grep -l "CrispyWaffle"
Length of output: 166
.github/workflows/deep-source.yml (1)
32-32: LGTM! Test command follows best practices.
The test command is well-structured with appropriate flags for coverage collection.
Tests/VTEX.Tests/VTEX.Tests.csproj (2)
18-18: LGTM! NSubstitute package addition looks good.
The addition of NSubstitute 5.3.0 is appropriate for the test project and aligns with the transition to a new mocking framework.
18-18: Verify complete transition from Moq
Let's ensure we're not maintaining two mocking frameworks simultaneously.
✅ Verification successful
Complete transition from Moq verified
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining Moq references or usages
# Check for Moq package reference in any .csproj file
echo "Checking for Moq package references..."
rg -l 'PackageReference.*Include="Moq"' -g '*.csproj'
# Check for Moq usages in test files
echo "Checking for Moq usages in test files..."
rg -l 'using Moq;|Mock<|\.Setup\(|\.Verify\(' -g '*.cs' Tests/
Length of output: 298
Script:
#!/bin/bash
# Description: Comprehensive verification for any remaining Moq references or usages across the entire codebase
# Search for Moq package references in all project and solution files
echo "Searching for Moq package references in .csproj and .sln files..."
rg -l 'PackageReference\s+Include\s*=\s*"Moq"' -g '*.csproj' -g '*.sln'
# Search for 'using Moq;' directives in all C# code files
echo "Searching for 'using Moq;' in C# files..."
rg -l '^using\s+Moq;' -g '*.cs'
# Search for Moq usage patterns (Mock<, .Setup(, .Verify()) in all C# code files
echo "Searching for Moq usage patterns (Mock<, .Setup(, .Verify()) in C# files..."
rg -l 'Mock<|\.Setup\(|\.Verify\(' -g '*.cs'
Length of output: 555
.github/workflows/codacy.yml (2)
54-55: Review max-allowed-issues configuration
Setting max-allowed-issues to 2147483647 effectively disables the threshold. Consider setting a reasonable threshold based on your project's quality standards, or document why this high value is necessary.
#!/bin/bash
# Check current issue count to help determine a reasonable threshold
gh api graphql -f query='
query {
repository(owner: "guibranco", name: "VTEX-SDK-dotnet") {
vulnerabilityAlerts(first: 1) {
totalCount
}
}
}'43-43: Consider using a major version tag instead of commit hash
While using a commit hash is secure, it requires manual updates. Consider using a major version tag (e.g., @v5) with SHA pinning in a separate workflow dependency file for better maintainability while maintaining security.
✅ Verification successful
Using a major version tag (e.g., @v4) is appropriate
The current latest version is v4.4.5, allowing you to switch to a major version tag for better maintainability while maintaining security with SHA pinning.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version of the action
gh api repos/codacy/codacy-analysis-cli-action/releases/latest --jq .tag_name
Length of output: 226
Script:
#!/bin/bash
# Check latest release tag of codacy-analysis-cli-action
gh api repos/codacy/codacy-analysis-cli-action/releases/latest --jq .tag_name
Length of output: 86
Tests/VTEX.Tests/Health/VtexHealthClientTests.cs (3)
21-21: LGTM! NSubstitute using directive added correctly.
The addition of the NSubstitute namespace import is appropriate for the mocking framework migration.
34-38: Documentation updated accurately to reflect NSubstitute usage.
The XML comments have been properly updated to use "substitute" terminology instead of "mock", maintaining consistency with the new mocking framework.
61-67: Verify the complete migration from Moq to NSubstitute.
The test implementation has been correctly updated to use NSubstitute syntax. However, let's ensure all Moq references have been removed from the project.
✅ Verification successful
Complete migration from Moq to NSubstitute verified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify complete migration from Moq to NSubstitute
# Check for any remaining Moq references in the codebase
# Search for Moq using directives
echo "Checking for remaining Moq using directives..."
rg "using\s+Moq" -g "*.cs"
# Search for Mock<T> usage
echo "Checking for remaining Mock<T> usage..."
rg "Mock<" -g "*.cs"
# Search for Moq package reference in project files
echo "Checking for Moq package references..."
rg "PackageReference.*Moq" -g "*.csproj"
Length of output: 373
| dotnet test -c Debug --verbosity minimal --no-build --no-restore /p:CollectCoverage=true /p:CoverletOutputFormat="cobertura" | ||
| ./bin/deepsource report --analyzer test-coverage --key csharp --value-file ./Tests/VTEX.Tests/coverage.cobertura.xml | ||
| dotnet test -c Debug --verbosity minimal --no-build --no-restore /p:CollectCoverage=true /p:CoverletOutputFormat="cobertura" | ||
| ./bin/deepsource report --analyzer test-coverage --key csharp --value-file ./Tests/VTEX.Tests/coverage.net8.0.cobertura.xml |
There was a problem hiding this comment.
💡 Codebase verification
Coverage reporting is only configured for .NET 8.0.
Coverage files for netstandard2.0, netstandard2.1, and net6.0 are missing.
🔗 Analysis chain
Verify coverage reporting for multiple target frameworks.
The coverage file path now includes the specific framework version (net8.0). If the project targets multiple frameworks, we should ensure this doesn't miss coverage from other target frameworks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if project targets multiple frameworks that might generate different coverage files
# Check target frameworks in the project file
echo "Checking target frameworks in VTEX.API.csproj:"
rg -A 2 "<TargetFrameworks|<DotNetVersions" Src/VTEX.API/VTEX.API.csproj
# Check if there are any other coverage files being generated
echo -e "\nChecking for other coverage file patterns:"
fd "coverage.*cobertura.xml" Tests/VTEX.Tests/
Length of output: 623
Script:
#!/bin/bash
# Description: Retrieve the value of DotNetVersions in VTEX.API.csproj
echo "Retrieving DotNetVersions from VTEX.API.csproj:"
rg "<DotNetVersions>(.*)</DotNetVersions>" Src/VTEX.API/VTEX.API.csproj --only-matching --replace '$1'
Length of output: 209
Script:
#!/bin/bash
# Description: Locate and retrieve the value of DotNetVersions across the repository
echo "Searching for the definition of DotNetVersions in the repository:"
rg "DotNetVersions.*=" --no-filename -A 1 .
Length of output: 186
Script:
#!/bin/bash
# Description: Comprehensive search for any occurrences of DotNetVersions in the repository
echo "Searching for all occurrences of DotNetVersions in the repository:"
rg "DotNetVersions" .
Length of output: 645
| "sonarlint.connectedMode.project": { | ||
| "connectionId": "guibranco-github", | ||
| "projectKey": "guibranco_VTEX-SDK-dotnet" | ||
| } |
There was a problem hiding this comment.
Fix JSON syntax error
There's a missing comma after the Peacock setting, which would cause the JSON to be invalid.
Apply this fix:
"peacock.color": "#e42760"
+ ,
"sonarlint.connectedMode.project": {
"connectionId": "guibranco-github",
"projectKey": "guibranco_VTEX-SDK-dotnet"
}📝 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.
| "peacock.color": "#e42760" | |
| "sonarlint.connectedMode.project": { | |
| "connectionId": "guibranco-github", | |
| "projectKey": "guibranco_VTEX-SDK-dotnet" | |
| } | |
| "peacock.color": "#e42760", | |
| "sonarlint.connectedMode.project": { | |
| "connectionId": "guibranco-github", | |
| "projectKey": "guibranco_VTEX-SDK-dotnet" | |
| } |
🧰 Tools
🪛 Biome
[error] 22-22: expected , but instead found "sonarlint.connectedMode.project"
Remove "sonarlint.connectedMode.project"
(parse)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "peacock.color": "#e42760" | ||
| } | ||
| "peacock.color": "#e42760" | ||
| "sonarlint.connectedMode.project": { |
Check warning
Code scanning / Jacksonlinter (reported by Codacy)
Unexpected character ('"' (code 34)): was expecting comma to separate Object entries Warning
| @@ -12,8 +12,11 @@ | |||
| contrib-readme-job: | |||
| runs-on: ubuntu-latest | |||
| name: Update files | |||
|
|
|||
| permissions: | |||
Check notice
Code scanning / Checkov (reported by Codacy)
Ensure top-level permissions are not set to write-all Note
| @@ -1,4 +1,4 @@ | |||
| name: Deploy Documentation | |||
| name: Publish Documentation | |||
Check notice
Code scanning / Checkov (reported by Codacy)
Ensure top-level permissions are not set to write-all Note
| @@ -23,4 +23,4 @@ | |||
| fetch-depth: 0 | |||
|
|
|||
| - name: Infisical secrets check | |||
| uses: guibranco/[email protected].15 | |||
| uses: guibranco/[email protected].16 | |||
Check warning
Code scanning / Semgrep (reported by Codacy)
An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Warning
|
❌ Build VTEX-SDK-dotnet 2.3.1730 failed (commit 8e7c8b4671 by @gstraccini[bot]) |
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 Footnotes
|
|
Infisical secrets check: ✅ No secrets leaked! 💻 Scan logs11:24AM INF scanning for exposed secrets...
11:24AM INF 585 commits scanned.
11:24AM INF scan completed in 301ms
11:24AM INF no leaks found
|
|
|
❌ Build VTEX-SDK-dotnet 2.3.1734 failed (commit f6f7e43d7b by @guibranco) |
Closes #
📑 Description
✅ Checks
☢️ Does this introduce a breaking change?
ℹ Additional Information
Summary by CodeRabbit
New Features
Updates
Bug Fixes