-
-
Notifications
You must be signed in to change notification settings - Fork 153
Adding cloudflare preview builds #822
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
Conversation
""" WalkthroughA new GitHub Actions workflow file named Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub PR as GitHub PR (to master)
participant Actions as GitHub Actions Runner
participant Cloudflare as Cloudflare Pages
participant PR as Pull Request
GitHub PR->>Actions: Trigger workflow on PR to master
Actions->>Actions: Checkout code, cache dependencies
Actions->>Actions: Setup Node.js & Yarn, install dependencies
Actions->>Actions: Build project (yarn build)
Actions->>Cloudflare: Deploy build output (src/dist)
Cloudflare-->>Actions: Return deployment URL
Actions->>PR: Comment with deployment URL and add reactions
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 2
🧹 Nitpick comments (1)
.github/workflows/deploy-preview.yml (1)
20-25
: Leveragesetup-node
built-in caching
Instead of manually cachingnode_modules
, you can combine Node.js setup and Yarn cache:- name: Install node.js (with cache) uses: actions/setup-node@v4 with: node-version-file: '.nvmrc' cache: 'yarn' cache-dependency-path: yarn.lockThis simplifies the workflow and speeds up dependency installs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-preview.yml
(1 hunks)
🔇 Additional comments (6)
.github/workflows/deploy-preview.yml (6)
1-1
: Workflow name is clear
Thename: 'Preview Deployment'
succinctly describes this workflow.
2-5
: Verify use of pull_request_target
Usingpull_request_target
grants access to secrets but runs the workflow against the base branch code. Ensure this is intentional and that no untrusted PR code can modify the workflow or exfiltrate secrets.
14-15
: No action required for runner and timeout
Theruns-on: ubuntu-latest
andtimeout-minutes: 5
settings are reasonable defaults; adjust only if your build routinely exceeds 5 minutes.
26-33
: No action required for Node.js and Yarn installation
The steps to install Node.js via.nvmrc
and then globally install Yarn are standard. Pinning Yarn to a specific version or using built-in cache flags is optional.
35-42
: Verify build output path
Confirm that your build actually emits files tosrc/dist
. If the output directory differs (e.g., a top-leveldist/
), update the deploy command accordingly:- pages deploy src/dist --project-name=... + pages deploy dist --project-name=...
43-51
: No action required for comment step
Usingactions-comment-pull-request
with reactions andmode: recreate
is a solid approach for surfacing the preview URL.
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
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.
Checkout PR head commit on pull_request_target
By default, actions/checkout
in a pull_request_target
event checks out the base branch. To build the PR changes, specify the head ref:
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.pull_request.head.sha }}
📝 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: Checkout code | |
uses: actions/checkout@v4 | |
- name: Checkout code | |
uses: actions/checkout@v4 | |
with: | |
ref: ${{ github.event.pull_request.head.sha }} |
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.
it was caught here and ignored
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch information from the referenced PRs to understand the context
# First PR
echo "Fetching information from PR #4468 on betaflight/betaflight-configurator"
gh api repos/betaflight/betaflight-configurator/pulls/4468 --jq '.title, .html_url, .body'
# Second PR
echo "Fetching information from PR #834 on betaflight/blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/pulls/834 --jq '.title, .html_url, .body'
# Look for discussions about pull_request_target in these PRs
echo "Looking for discussions about pull_request_target in PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[].body' | grep -i "pull_request_target" || echo "No mentions found in comments"
echo "Looking for discussions about pull_request_target in PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[].body' | grep -i "pull_request_target" || echo "No mentions found in comments"
# Look for review comments that might mention this issue
echo "Looking for review comments about pull_request_target in PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[].body' | grep -i "checkout" || echo "No mentions of checkout in comments"
echo "Looking for review comments about pull_request_target in PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[].body' | grep -i "checkout" || echo "No mentions of checkout in comments"
Length of output: 3278
🏁 Script executed:
#!/bin/bash
# Let's check if those PRs were actually merged or closed
echo "Checking status of PR #4468 on betaflight/betaflight-configurator"
gh api repos/betaflight/betaflight-configurator/pulls/4468 --jq '.state, .merged'
echo "Checking status of PR #834 on betaflight/blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/pulls/834 --jq '.state, .merged'
# Let's also check if there were any discussions from the maintainers
echo "Looking for comments from maintainers on PR #4468"
gh api repos/betaflight/betaflight-configurator/pulls/4468/comments --jq '.[] | {user: .user.login, body: .body}' | grep -A 2 -i "blckmn\|nerdCopter"
echo "Looking for comments from maintainers on PR #834"
gh api repos/betaflight/blackbox-log-viewer/pulls/834/comments --jq '.[] | {user: .user.login, body: .body}' | grep -A 2 -i "blckmn\|nerdCopter"
# Let's see if there are any workflow files using pull_request_target in these repos
echo "Checking for workflow files using pull_request_target in blackbox-log-viewer"
gh api repos/betaflight/blackbox-log-viewer/contents/.github/workflows --jq '.[] | .name'
Length of output: 1256
@nerdCopter Thank you for providing that context. I see that both PRs you referenced have the same concern identified but remain open. I understand that the team is aware of the security implications with the current approach using pull_request_target
and has made a deliberate decision about how to handle it. I'll respect that decision and won't flag this issue in future reviews.
✏️ Learnings added
Learnt from: nerdCopter
PR: betaflight/blackbox-log-viewer#822
File: .github/workflows/deploy-preview.yml:17-19
Timestamp: 2025-05-14T16:08:27.243Z
Learning: For betaflight/blackbox-log-viewer repository, the team is aware of the security implications of using `pull_request_target` without specifying `persist-credentials: false` in the checkout action, and has chosen to proceed with the current approach. This concern should not be flagged in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
permissions: | ||
actions: read | ||
contents: read | ||
deployments: write | ||
pull-requests: write |
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.
🛠️ Refactor suggestion
Add issues: write
permission for PR comments
To allow the comment action to post deployment URLs on the pull request, include the issues: write
permission under permissions
.
permissions:
actions: read
contents: read
deployments: write
+ issues: write
pull-requests: write
📝 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.
permissions: | |
actions: read | |
contents: read | |
deployments: write | |
pull-requests: write | |
permissions: | |
actions: read | |
contents: read | |
deployments: write | |
issues: write | |
pull-requests: write |
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.
adding this to my PR's as well
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.
@nerdCopter Great! Adding the issues: write
permission will ensure your workflow can properly post comments on pull requests. Let me know if you need any other assistance with your GitHub Actions workflows.
✏️ Learnings added
Learnt from: nerdCopter
PR: betaflight/blackbox-log-viewer#822
File: .github/workflows/deploy-preview.yml:9-13
Timestamp: 2025-05-14T16:07:59.143Z
Learning: nerdCopter prefers to include `issues: write` permission in GitHub Actions workflows that post comments on pull requests, in addition to the standard `pull-requests: write` permission.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
with: | ||
message: | | ||
Preview URL: ${{ steps.deploy.outputs.deployment-url }} | ||
reactions: eyes, rocket |
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.
reactions: eyes, rocket |
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.
@blckmn - this was requested by @nerdCopter
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.
Yes this updates the single comment (recreation mode) issue.
I'm keeping the emojis though :)
|
This pull request introduces a new GitHub Actions workflow to automate preview deployments for pull requests targeting the
master
branch. The workflow includes steps for building the project, deploying to Cloudflare, and commenting the preview URL back on the pull request.New GitHub Actions Workflow for Preview Deployments:
.github/workflows/deploy-preview.yml
: Added a new workflow namedPreview Deployment
that triggers on pull requests targeting themaster
branch. The workflow includes steps for caching dependencies, installing Node.js, building the project with Yarn, deploying to Cloudflare using the Wrangler action, and commenting the preview URL on the pull request.Summary by CodeRabbit