-
Notifications
You must be signed in to change notification settings - Fork 85
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
chore: fix sonarqube scan #1995
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the Changes
The changes are minimal and focused on path variable modifications in the shell script, which likely impact how file paths are processed during report generation in GitHub runner environments. 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 review |
✅ Actions performedReview triggered.
|
Quality Gate passedIssues Measures |
@coderabbitai review |
✅ Actions performedReview triggered.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (1)
scripts/fix-reports-path-in-github-runner.sh (1)
4-4
: Remove unused variabledefaultAbsolutePathPrefix
.The
defaultAbsolutePathPrefix
variable is declared but never used in the script. If it's not needed for external use, consider removing it to improve code maintainability.-defaultAbsolutePathPrefix="home/runner/work/rudder-sdk-js/rudder-sdk-js/"
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 4-4: defaultAbsolutePathPrefix appears unused. Verify use (or export if used externally).
(SC2034)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/unit-tests-and-lint.yml
is excluded by!**/*.yml
package.json
is excluded by!**/*.json
📒 Files selected for processing (1)
scripts/fix-reports-path-in-github-runner.sh
(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
scripts/fix-reports-path-in-github-runner.sh
[warning] 4-4: defaultAbsolutePathPrefix appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Bundle size checks
🔇 Additional comments (2)
scripts/fix-reports-path-in-github-runner.sh (2)
3-3
: Verify the empty prefix replacement with SonarQube documentation.The removal of the "/github/workspace" prefix suggests a change in path handling for the new SonarQube action. Please ensure this aligns with the official documentation for the new
sonarqube-scan-action
.✅ Verification successful
Empty prefix replacement is correct for path normalization
The empty prefix is the right approach as the script's purpose is to strip absolute paths from coverage, eslint, and sonar reports, making them portable across different environments. The script correctly handles both default and self-hosted runner paths, ensuring consistent report paths for SonarQube analysis.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the new SonarQube action documentation mentions path handling changes gh api repos/SonarSource/sonarqube-scan-action/contents/README.md | jq -r '.content' | base64 -d | grep -A 5 -B 5 "github/workspace"Length of output: 227
Script:
#!/bin/bash # Find references to the script and its usage rg -l "fix-reports-path-in-github-runner" --type yaml --type sh # Show the content of the script to understand the full context cat scripts/fix-reports-path-in-github-runner.sh # Look for test files related to this script fd test.*sh$ scripts/Length of output: 1084
4-5
: Review path concatenation in sed commands.The addition of trailing slashes to path prefixes combined with leading slashes in some sed patterns could lead to double slashes (e.g.,
//
). While this might work, it's better to maintain consistent path handling.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 4-4: defaultAbsolutePathPrefix appears unused. Verify use (or export if used externally).
(SC2034)
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1995 +/- ##
========================================
Coverage 61.27% 61.27%
========================================
Files 484 484
Lines 16616 16616
Branches 3329 3331 +2
========================================
Hits 10182 10182
- Misses 5225 5243 +18
+ Partials 1209 1191 -18 ☔ View full report in Codecov by Sentry. |
PR Description
I've used the latest SonarQube action in the workflow as suggested by Sonar.
Also, paths in the reports are fixed per the latest action.
Additionally, optimised the tests workflow by not building the packages.
The warning we used to get in the workflows:
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-2826/move-error-reporting-functionality-to-the-core-sdk
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit