Add new workflow for updating VR snapshots#6541
Conversation
WalkthroughTwo workflow files modified: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Trigger as Manual Dispatch or<br/>PR Comment with /update-snapshots
participant Workflow as Update VR<br/>Snapshots Workflow
participant Git as Git & GitHub API
participant Docker as Docker Build<br/>& Test
Trigger->>Workflow: Trigger event
alt Issue Comment Trigger
Workflow->>Git: Fetch PR metadata<br/>(head ref, SHA, state, base)
else Manual Dispatch
Note over Workflow: Use default context
end
Workflow->>Git: Checkout repository<br/>(full history, PR branch if applicable)
Workflow->>Docker: Setup Node.js, install deps
Workflow->>Docker: Build Docker image
Workflow->>Docker: Run VR snapshot updates
Workflow->>Git: Detect changes via git status
alt Changes Detected
Workflow->>Git: Configure Git user
alt On main branch
Workflow->>Git: Create new branch
Workflow->>Git: Commit & push changes
Workflow->>Git: Create PR against main
else On open PR
Workflow->>Git: Commit & push on PR branch
alt Triggered by comment
Workflow->>Git: Post success comment on PR
end
else On closed PR
Workflow->>Git: Create new branch from base
Workflow->>Git: Commit & push changes
Workflow->>Git: Create PR against original base
Workflow->>Git: Comment new PR URL on original PR
end
else No Changes
alt Triggered by comment
Workflow->>Git: Post informational comment on PR
else Manual trigger
Workflow->>Workflow: Log no-op message
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (2)
.github/workflows/update-vr-snapshots.yml (2)
118-124: Specify branch explicitly ingit pushfor robustness.Line 123 uses
git pushwithout specifying the branch. While this may work if git tracking is configured, it's safer and more explicit to specify the branch or usegit push origin HEAD:refs/heads/$(git symbolic-ref --short HEAD)or simplygit push -u origin HEAD. This ensures the push succeeds even if tracking is not configured or the branch was just created in the same job.Apply this diff:
- name: Commit and Push on PR Branch if: steps.check_changes.outputs.has_changes == 'true' && github.event_name == 'issue_comment' && steps.get_pr_branch.outputs.is_open == 'true' run: | git add . git commit -m "chore: update visual regression snapshots" - git push + git push origin HEAD
125-142: Usegh pr commentfor consistency with ci.yml.Line 142 uses
gh issue commentto comment on the original PR. While this works (PRs are issues in GitHub's model), ci.yml line 284 usesgh pr commentfor the same purpose. For consistency, semantic clarity, and reduced confusion, usegh pr comment:# Comment on the original closed PR with link to new PR - gh issue comment ${{ github.event.issue.number }} --body "✅ Visual regression snapshots have been updated in a new PR: $PR_URL" + gh pr comment ${{ github.event.issue.number }} --body "✅ Visual regression snapshots have been updated in a new PR: $PR_URL"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/update-vr-snapshots.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.549Z
Learning: Applies to test-vr/__snapshots__/** : Commit newly recorded or updated visual regression snapshot files under test-vr/__snapshots__ to the repository
📚 Learning: 2025-10-25T07:34:46.549Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.549Z
Learning: Applies to test-vr/__snapshots__/** : Commit newly recorded or updated visual regression snapshot files under test-vr/__snapshots__ to the repository
Applied to files:
.github/workflows/update-vr-snapshots.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
.github/workflows/ci.yml (1)
278-281: Instruction text integrates well with VR failure reporting.The added separator and
/update-snapshotsinstruction text clearly guides users to the new workflow. The formatting and placement in the comment are appropriate..github/workflows/update-vr-snapshots.yml (9)
60-68: Verify ref checkout evaluation logic with conditional inputs.The ref parameter uses a ternary operator that depends on step outputs only available when
github.event_name == 'issue_comment':ref: ${{ github.event_name == 'issue_comment' && steps.get_pr_branch.outputs.ref || github.ref }}When triggered via
workflow_dispatch,steps.get_pr_branchdoes not execute, sosteps.get_pr_branch.outputs.refmay not exist. While the short-circuit AND operator (&&) prevents evaluation if the first condition is false, this pattern could be fragile. Ifget_pr_branch.outputs.refevaluates to an empty string on issue_comment triggers (due to timing or other issues), it would silently fall back togithub.ref.Consider using explicit conditionals for clarity:
- name: Checkout Repository uses: actions/checkout@v4 with: fetch-depth: 0 token: ${{ secrets.GITHUB_TOKEN }} ref: ${{ steps.get_pr_branch.outputs.ref || github.ref }}This relies on the GitHub Actions context to only populate the outputs when the step runs, which is more explicit. Alternatively, verify that the current pattern handles the
workflow_dispatchtrigger correctly by testing with that trigger to ensuregithub.refis used as fallback.
85-101: Change detection and git configuration are well-guarded.The workflow correctly detects changes and only configures git when needed. All commit/push steps properly check the
has_changesflag, ensuring git config runs before any git operations.
25-29: Trigger condition guards correctly against non-PR comments.The workflow condition properly checks that issue_comment events are only processed when they occur on a PR (
github.event.issue.pull_request) and contain the/update-snapshotsphrase. This prevents unnecessary workflow runs.
102-142: Three execution paths are well-designed and properly separated.The workflow correctly handles three scenarios: automatic snapshot updates on main (creates new PR), updates on open PRs (commits to existing branch), and updates on closed PRs (creates new PR and comments original). Each path has appropriate logic and messaging. The timestamped branch names prevent collisions if multiple updates occur.
1-19: Workflow structure and environment setup are sound.The workflow uses appropriate Node.js version pinning (20.x), proper permissions for contents and PRs, and standard GitHub Actions. The trigger setup correctly distinguishes between manual dispatch and comment-based automation.
31-41: Emoji reaction provides good UX feedback.The workflow reacts to the triggering comment with a rocket emoji, giving users immediate feedback that their request was recognized and is processing.
43-58: PR data fetching via github-script is correct and comprehensive.The workflow fetches all necessary PR details (head ref, head SHA, open state, base branch) needed for the three execution scenarios. Step only runs for issue_comment triggers, avoiding unnecessary API calls.
82-83: Snapshot update and Docker preparation steps are appropriately minimal.The workflow delegates to npm run scripts (
test-vr:prepareandtest-vr:update), maintaining separation of concerns and keeping the workflow maintainable. These scripts are standard in the recharts repo based on ci.yml.
144-166: Comment strategy provides clear feedback to users.The workflow offers three distinct messages: success with snapshots updated (145-154), no changes needed (156-166), and creation of new PR for closed PRs (142). This keeps users informed at each step.
|
/update-snapshots |
|
sick, now I can be even more lazy |
|
Hmmm that didn't work. I will try to merge it and see if it runs from main. |
2 participants
I was going to test it on the failed VR test that made it to main branch but you fixed it faster Coltin. I don't want to run the VR update on pre-push because it takes 10+ minutes sometimes.
Summary by CodeRabbit
New Features
/update-snapshotscomment on pull requestsDocumentation