Skip to content

[#864] Reduce PR CI cost by removing visual regression job#869

Merged
realproject7 merged 2 commits intomainfrom
task/864-reduce-pr-ci-cost
Apr 12, 2026
Merged

[#864] Reduce PR CI cost by removing visual regression job#869
realproject7 merged 2 commits intomainfrom
task/864-reduce-pr-ci-cost

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Removed visual-regression job from PR CI — it remains available as manual-only via Update Visual Snapshots workflow (update-snapshots.yml)
  • Added concurrency group with cancel-in-progress: true so superseded PR CI runs are auto-canceled
  • PR CI now runs only lint-and-typecheck and e2e (no app runtime changes)

Changes

  • .github/workflows/ci.yml: removed visual-regression job (25 lines), added concurrency block (4 lines)

Test plan

  • Open a PR → CI should run only lint-and-typecheck and e2e jobs
  • Push again to same PR → previous CI run should be canceled
  • Update Visual Snapshots workflow remains available for manual dispatch
  • No app runtime code changed

Fixes #864

🤖 Generated with Claude Code

- Drop the visual-regression job from ci.yml (remains manual-only in
  update-snapshots.yml)
- Add concurrency group so superseded PR runs are auto-canceled
- PR CI now runs only lint-and-typecheck + e2e

Fixes #864

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Apr 12, 2026 1:14pm

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The ci.yml change is aligned with part of the ticket, but the PR does not include the other workflow/doc updates explicitly required by issue #864.

Findings

  • [high] The issue scope calls for updating .github/workflows/update-snapshots.yml and CLAUDE.md in addition to ci.yml, but this PR changes only .github/workflows/ci.yml.
    • File: .github/workflows/ci.yml:1
    • Suggestion: Include the prepared update-snapshots.yml concurrency/timeout changes and the CLAUDE.md guidance update, or narrow the issue/acceptance criteria before merging.

Decision

Requesting changes because the PR does not satisfy the full accepted scope for #864 as written.

- Add concurrency group and 15-min timeout to update-snapshots.yml
- Update CLAUDE.md with visual regression manual-only guidance and
  updated repo structure listing both workflow files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The PR now includes the full accepted scope for #864 and keeps the change set limited to workflow/doc updates only.

Findings

  • No further findings.

Decision

Approving. PR CI is reduced to lint-and-typecheck plus e2e, visual regression remains manual-only with concurrency/timeout in the dedicated workflow, and no runtime app files are changed.

@realproject7 realproject7 merged commit 2a6ff9e into main Apr 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce PR CI cost by moving visual regression to manual workflow

2 participants