Don't verify when pushing, add debug by PavelVanecek · Pull Request #6548 · recharts/recharts

PavelVanecek · 2025-11-01T08:39:22Z

The snapshot update script ALMOST worked but it had two small problems:

when pushing it tried to run tests but failed because it didn't have all dependencies installed -> I made it skip tests because the only purpose is to update snapshots, it need not bother running tests
it tried to open a new PR even though it was supposed to push a commit to the same PR -> added condition

Summary by CodeRabbit

Chores
- Updated automated snapshot handling workflow with improved PR update management.
- Enhanced automated messaging on pull requests to communicate update status.

coderabbitai · 2025-11-01T08:39:34Z

Walkthrough

This PR modifies the GitHub Actions workflow for updating visual regression test snapshots by adding a debugging step to expose runtime event details, extending PR metadata to include base_branch, refactoring the main branch commit and push logic to create pull requests instead, introducing no-verify flags for commits and pushes, and adding a dedicated flow with messaging for handling updates on closed PRs.

Changes

Cohort / File(s)	Summary
GitHub Actions Workflow Updates `.github/workflows/update-vr-snapshots.yml`	Adds Print Event Info step for debugging; ensures base_branch emission from PR data; replaces main branch commit-and-push with conditional PR creation flow using workflow_dispatch; introduces no-verify flags for commits and pushes across PR and main branch paths; adds dedicated flow for creating PRs on closed PR updates; includes new PR messaging step to confirm updates or report no changes; adjusts formatting and spacing around modified steps.

Sequence Diagram(s)

sequenceDiagram
    actor GitHub
    participant Workflow as update-vr-snapshots Workflow
    participant VR as Visual Regression Tests
    participant Git as Git Operations
    participant PR as PR Creation

    GitHub->>Workflow: Trigger (workflow_dispatch or PR)
    Workflow->>Workflow: Print Event Info (new)
    Workflow->>VR: Run VR Tests
    
    alt Updates Generated
        alt On Main Branch
            VR->>Git: Commit (--no-verify)
            Git->>PR: Create PR to main
            PR->>Workflow: PR Created
            Workflow->>Workflow: Comment on PR
        else On PR Branch
            VR->>Git: Commit (--no-verify)
            Git->>Git: Push (--no-verify)
            Workflow->>Workflow: Comment on PR
        else On Closed PR
            VR->>Git: Commit (--no-verify)
            Git->>PR: Create new PR
            PR->>Workflow: PR Created
            Workflow->>Workflow: Comment on new PR
        end
    else No Updates
        Workflow->>Workflow: Comment: No changes
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Control flow complexity: Multiple conditional branches for different scenarios (main branch dispatch, PR updates, closed PR handling) require verification of branching logic and conditional triggers
No-verify flag usage: Ensure no-verify flags are applied consistently and appropriately across all commit/push operations
PR creation logic: Validate new PR creation flow on main branch and closed PR scenarios works as intended
Event data changes: Verify base_branch emission and event info step properly expose and utilize GitHub event context

Possibly related PRs

Add new workflow for updating VR snapshots #6541: Directly related refactoring of the same workflow file with equivalent changes to event debugging, PR data emission, commit/push control flow, and PR messaging.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description is largely incomplete when compared against the required template. While the author provides a brief explanation of two key fixes (skipping tests during push operations and fixing incorrect PR creation logic), the description is missing several required sections including Related Issue, Motivation and Context details, How Has This Been Tested, Types of changes checkboxes, and the required Checklist. The provided text does not follow the template structure and lacks the necessary formality and completeness expected for a pull request to this repository. Only a minimal subset of critical information is present.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Don't verify when pushing, add debug" is concise and clearly relates to the main changes in the pull request. It captures the two primary modifications: using the `--no-verify` flag when pushing commits (to skip pre-push hooks/tests) and adding debug output to expose runtime GitHub event details. The title is specific enough that a reviewer scanning the repository history would understand the core focus of these workflow script updates. While it doesn't enumerate every change in the changeset, this is acceptable as the title successfully highlights the most important modifications from the developer's perspective.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch noverify

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 770206f and 0c9c0be.

📒 Files selected for processing (1)

.github/workflows/update-vr-snapshots.yml (6 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.558Z
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.558Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.558Z
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). (1)

GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (9)

.github/workflows/update-vr-snapshots.yml (9)

1-5: Good documentation of --no-verify intent.

The comment on line 5 clearly explains why git hooks are bypassed during snapshot updates. This helps future maintainers understand the reasoning.

44-51: Debug output aids troubleshooting.

The Print Event Info step provides useful context for diagnosing workflow behavior across different trigger scenarios (workflow_dispatch vs. issue_comment). This is appropriate for CI/CD logging and helps when investigating failures.

53-68: Correct PR metadata extraction and base_branch propagation.

The addition of base_branch output (line 68) is necessary for correctly routing closed PR updates to their original base branch (line 151). The conditional limit to issue_comment ensures outputs are only used where the step has run.

112-127: Main branch PR creation correctly uses --no-verify.

The workflow properly creates a new branch, commits with --no-verify, and pushes with --no-verify to avoid running git hooks. This aligns with the PR objectives. The condition correctly restricts this flow to workflow_dispatch on main branch only.

129-135: PR branch update correctly commits and pushes to existing PR.

The condition properly restricts this to open PRs (via issue_comment trigger), and the workflow correctly uses --no-verify on both commit and push operations per the PR objectives.

137-155: Closed PR flow correctly creates new PR and notifies original author.

This flow properly handles the edge case where snapshots are requested on a closed PR. It creates a new PR against the original's base branch (not hardcoded to main), uses --no-verify correctly, and comments back on the original PR to inform the user. Good UX design.

157-167: Good explicit feedback for PR branch updates.

The condition correctly targets open PRs updated via issue_comment, providing explicit confirmation feedback. This creates good UX parity with the no-changes message (line 170).

169-179: Clear feedback provided when no changes detected.

The messaging appropriately informs users when snapshots are already up to date, maintaining good UX for both update and no-change scenarios.

1-179: Workflow logic is sound and objectives are met.

The refactored workflow correctly implements the PR objectives: (1) uses --no-verify on all git operations to skip hooks/tests (lines 121-122, 134-135, 146-147), and (2) conditionally pushes to existing PR branches (line 131) vs. creating new PRs (lines 113-127, 138-155). The three distinct paths (main branch, open PR, closed PR) are properly gated by mutually exclusive conditions, and PR metadata (base_branch) is correctly propagated to support the closed PR flow.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

PavelVanecek added 2 commits November 1, 2025 17:34

Don't verify when pushing, add debug

baada32

github.event_name == 'workflow_dispatch'

0c9c0be

PavelVanecek merged commit 5fd1d87 into main Nov 1, 2025
6 of 9 checks passed

PavelVanecek deleted the noverify branch November 1, 2025 08:46

PavelVanecek mentioned this pull request Nov 1, 2025

Allow overwriting Label textAnchor prop #6547

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Don't verify when pushing, add debug#6548

Don't verify when pushing, add debug#6548
PavelVanecek merged 2 commits into
mainfrom
noverify

PavelVanecek commented Nov 1, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

PavelVanecek commented Nov 1, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

PavelVanecek commented Nov 1, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 1, 2025 •

edited

Loading