Conversation
WalkthroughChanged CI runners and added workspace cleanup steps; added a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/tests.yaml (1)
21-24: Harden cleanup to anchor deletion to${GITHUB_WORKSPACE}.The current glob-based
sudo rm -rfworks, but it's safer to anchor deletion to${GITHUB_WORKSPACE}so an unexpected working-directory change can't broaden the removal scope. This pattern appears in multiple workflow files and should be updated consistently.Safer cleanup variant
- name: Cleanup build folder run: | - sudo rm -rf ./* || true - sudo rm -rf ./.??* || true + test -n "${GITHUB_WORKSPACE:-}" && test -d "${GITHUB_WORKSPACE}" + sudo rm -rf -- "${GITHUB_WORKSPACE:?}/"* \ + "${GITHUB_WORKSPACE:?}"/.[!.]* \ + "${GITHUB_WORKSPACE:?}"/..?* || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/tests.yaml around lines 21 - 24, The "Cleanup build folder" step uses unanchored globs (rm -rf ./* and ./.??*) which can delete outside the intended directory if the working dir changes; update this step to perform removals anchored to the workspace variable (use ${GITHUB_WORKSPACE:?} as the base) so both normal and dotfiles are removed from ${GITHUB_WORKSPACE} only, and apply the same change to the other workflow cleanup occurrences to ensure consistent, safe deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/tests.yaml:
- Around line 21-24: The "Cleanup build folder" step uses unanchored globs (rm
-rf ./* and ./.??*) which can delete outside the intended directory if the
working dir changes; update this step to perform removals anchored to the
workspace variable (use ${GITHUB_WORKSPACE:?} as the base) so both normal and
dotfiles are removed from ${GITHUB_WORKSPACE} only, and apply the same change to
the other workflow cleanup occurrences to ensure consistent, safe deletion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce202099-3d6a-4fc8-8f10-43ead66dc43c
📒 Files selected for processing (3)
.github/workflows/tests.yamlx/deployment/keeper/grpc_query.gox/market/keeper/grpc_query.go
✅ Files skipped from review due to trivial changes (2)
- x/market/keeper/grpc_query.go
- x/deployment/keeper/grpc_query.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/setup-ubuntu/action.yaml:
- Around line 3-7: Change the inputs.go-cache default from 'false' to 'true' in
the action definition so actions/setup-go module cache restore/save is enabled
by default; update the inputs block for the go-cache input (symbol: go-cache,
key: default) to 'true' and ensure no other behavior relies on the current false
default (leave required/description unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 358863df-6b4d-440e-a7ea-3888d13dbc6a
📒 Files selected for processing (2)
.github/actions/setup-ubuntu/action.yaml.github/workflows/tests.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/tests.yaml:
- Around line 21-24: The cleanup step named "Cleanup build folder" currently
masks failures by appending "|| true" to the rm commands (sudo rm -rf ./* ||
true and sudo rm -rf ./.??* || true); remove the "|| true" and replace the
brittle suppression with a safe existence check or a find-based removal so
failures surface — e.g. wrap removals in conditionals (if [ -e ./* ] || [ -e
./.??* ]; then sudo rm -rf ./* ./.??*; fi) or use a tolerant deletion like find
. -maxdepth 1 -mindepth 1 -exec sudo rm -rf {} +; apply the same change to the
other occurrence (the second pair of rm commands) so cleanup fails fast instead
of being silently ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 415672a4-33ae-4550-8fdf-e14176ce3cb7
📒 Files selected for processing (1)
.github/workflows/tests.yaml
| - name: Cleanup build folder | ||
| run: | | ||
| sudo rm -rf ./* || true | ||
| sudo rm -rf ./.??* || true |
There was a problem hiding this comment.
Don’t suppress cleanup failures with || true.
Line 23/24 and Line 51/52 currently mask cleanup errors. That can silently keep stale state on persistent runners and reintroduce flaky builds.
Safer fail-fast cleanup
- - name: Cleanup build folder
- run: |
- sudo rm -rf ./* || true
- sudo rm -rf ./.??* || true
+ - name: Cleanup build folder
+ run: |
+ sudo find . -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +Also applies to: 49-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/tests.yaml around lines 21 - 24, The cleanup step named
"Cleanup build folder" currently masks failures by appending "|| true" to the rm
commands (sudo rm -rf ./* || true and sudo rm -rf ./.??* || true); remove the
"|| true" and replace the brittle suppression with a safe existence check or a
find-based removal so failures surface — e.g. wrap removals in conditionals (if
[ -e ./* ] || [ -e ./.??* ]; then sudo rm -rf ./* ./.??*; fi) or use a tolerant
deletion like find . -maxdepth 1 -mindepth 1 -exec sudo rm -rf {} +; apply the
same change to the other occurrence (the second pair of rm commands) so cleanup
fails fast instead of being silently ignored.
|
Marked as stale; will be closed in five days. |
1 participant
Description
!!! Do not merge. Testing GH runners !!!
Notes:
actions/setup-gocache saves few minutes each run, on hosted runners as cache pull speed is slow (~5-6Mb/s)rootownership. It requires to cleanup build folder before running job.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.md