-
Notifications
You must be signed in to change notification settings - Fork 108
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
ci: skip cache build for dependabot PRs #3270
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
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
🧹 Outside diff range and nitpick comments (2)
.github/workflows/e2e.yml (2)
Line range hint
156-157
: Track technical debt: Plan V2 migration cleanupThese TODO comments reference issue #2627 for V2 migration. Consider creating a milestone or project board item to track the removal of these temporary conditions after the V2 migration is complete.
Line range hint
284-341
: Consider refactoring the e2e-ok job for better maintainabilityThe current implementation mixes concerns between Slack notification and status checking. Consider these improvements:
- Extract Slack notification to a separate composite action:
- - name: Send slack message with results - uses: actions/github-script@v7 - if: ${{ github.event_name == 'schedule' || (github.event_name == 'push' && needs.e2e.result == 'failure') }} - env: - SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_CI_ALERTS }} - with: - script: | - const {data} = await github.rest.actions.listJobsForWorkflowRunAttempt({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: context.runId, - attempt_number: ${{ github.run_attempt }}, - }); - # ... rest of the script ... + - name: Send test results to Slack + uses: ./.github/actions/notify-slack-test-results + if: ${{ github.event_name == 'schedule' || (github.event_name == 'push' && needs.e2e.result == 'failure') }} + with: + webhook-url: ${{ secrets.SLACK_WEBHOOK_CI_ALERTS }} + workflow-run-id: ${{ github.run_id }} + workflow-attempt: ${{ github.run_attempt }}
- Simplify the exit code logic:
- - run: | - result="${{ needs.build-zetanode.result }}" - if [[ $result == "failed" ]]; then - exit 1 - fi - result="${{ needs.e2e.result }}" - if [[ $result == "success" || $result == "skipped" ]]; then - exit 0 - else - exit 1 - fi + - run: | + [[ "${{ needs.build-zetanode.result }}" != "failed" ]] && + [[ "${{ needs.e2e.result }}" =~ ^(success|skipped)$ ]]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/e2e.yml
(1 hunks)
🔇 Additional comments (2)
.github/workflows/e2e.yml (2)
Line range hint 251-283
: LGTM: Matrix strategy effectively manages test execution
The implementation follows GitHub Actions best practices:
- Uses fail-fast: false to prevent single test failure from stopping other tests
- Configures appropriate runners and timeouts for resource-intensive tasks
- Properly inherits secrets and uses reusable workflow pattern
35-35
: LGTM: Condition correctly excludes Dependabot PRs from cache builds
The condition github.actor != 'dependabot[bot]'
is properly combined with the existing fork check, ensuring that Dependabot PRs fall back to the slow build process, which aligns with the PR objectives.
Let's verify the condition works as expected:
✅ Verification successful
Condition correctly implemented to skip cache builds for Dependabot PRs
The workflow implementation has been verified to work as intended:
- The condition
github.actor != 'dependabot[bot]'
is correctly placed in thebuild-zetanode
job - Recent workflow runs show Dependabot PRs are being processed, with the cache build being skipped
- The condition is properly combined with the existing fork check logic, maintaining security constraints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the workflow behavior for Dependabot PRs
# Expected: Workflow should skip build-zetanode for Dependabot PRs
# Check recent workflow runs for Dependabot PRs
gh run list --workflow=e2e.yml --json conclusion,displayTitle,headBranch,databaseId --jq '.[] | select(.headBranch | contains("dependabot")) | {conclusion, displayTitle, headBranch, url: "https://github.com/zeta-chain/node/actions/runs/\(.databaseId)"}' --limit 10
Length of output: 893
Script:
#!/bin/bash
# Let's examine the workflow file to verify the condition's implementation
cat .github/workflows/e2e.yml
Length of output: 16566
Skip build-zetanode step for depdenabot PRs as dependabot cannot push to ghcr.io. dependabot PRs will just use the slow build like fork PRs do.
reference.
Related to #3269
Summary by CodeRabbit
New Features
Bug Fixes
Documentation