Skip to content

Conversation

@daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Nov 26, 2025

Greptile Overview

Greptile Summary

Removed GitHub Actions logic that triggered full test suite re-runs when PRs were approved. Previously, the workflow listened to pull_request_review events and would re-execute tests when a reviewer approved the PR. This change eliminates that behavior by:

  • Removing the approval status checking step that used GitHub API to determine if a PR was approved
  • Removing the pull_request_review event handling logic that ran tests on approval
  • Removing the approval state from the pr-requirements-pass job condition
  • Simplifying Pandoc installation in the CLI workflow by removing dead code (conditional checks for non-existent matrix strategy)

The binary test file tests/sims/full_fdtd.h5 was regenerated as part of test maintenance.

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - it simplifies CI/CD behavior and removes unnecessary workflow runs
  • The changes correctly achieve the stated goal of avoiding re-runs on approval. The workflow will still trigger on PR synchronize events (new pushes), which ensures tests run when code changes. The Pandoc cleanup removes dead code. One minor concern is the unintentional removal of push-to-develop test triggers, which may have been used elsewhere in the workflow.
  • Verify that removing push-to-develop triggers in .github/workflows/tidy3d-python-client-tests.yml doesn't break expected CI behavior

Important Files Changed

File Analysis

Filename Score Overview
.github/workflows/tidy3d-python-client-tests.yml 4/5 Removed approval-checking logic and pull_request_review trigger handling; simplified test execution to avoid re-running on PR approval
.github/workflows/tidy3d-python-client-develop-cli.yml 3/5 Consolidated Pandoc installation to Ubuntu-only, removing cross-platform support for macOS and Windows

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant PR as Pull Request
    participant GHA as GitHub Actions
    participant Tests as Test Suite
    
    Note over Dev,Tests: Before (with approval re-run)
    Dev->>PR: Push new commit
    PR->>GHA: Trigger pull_request event
    GHA->>Tests: Run local tests
    Tests-->>GHA: Tests complete
    
    Dev->>PR: Request review
    PR->>GHA: Trigger pull_request_review (approved)
    GHA->>GHA: Check approval status
    GHA->>Tests: Re-run full test suite (local + remote)
    Tests-->>GHA: Tests complete again
    
    Note over Dev,Tests: After (this PR's changes)
    Dev->>PR: Push new commit
    PR->>GHA: Trigger pull_request event
    GHA->>Tests: Run local tests
    Tests-->>GHA: Tests complete
    
    Dev->>PR: Request review
    Note over PR,GHA: No workflow trigger on approval
    Note over GHA,Tests: Tests not re-run unnecessarily
Loading

@daquinteroflex daquinteroflex force-pushed the dario/approval_not_rerun branch from 9a08f08 to 8db8a8e Compare November 26, 2025 12:31
@daquinteroflex daquinteroflex marked this pull request as ready for review November 26, 2025 12:32
@daquinteroflex daquinteroflex changed the title chore: avoid rerun on approval chore: avoid rerun on approval (FXC-3911) Nov 26, 2025
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@daquinteroflex daquinteroflex added this pull request to the merge queue Nov 26, 2025
@github-actions
Copy link
Contributor

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Nov 26, 2025
@daquinteroflex daquinteroflex force-pushed the dario/approval_not_rerun branch from 8db8a8e to b00a5ad Compare November 26, 2025 13:11
@daquinteroflex daquinteroflex force-pushed the dario/approval_not_rerun branch from b00a5ad to e8e8023 Compare November 26, 2025 13:11
@daquinteroflex daquinteroflex added this pull request to the merge queue Nov 26, 2025
Merged via the queue into develop with commit 5230180 Nov 26, 2025
19 checks passed
@daquinteroflex daquinteroflex deleted the dario/approval_not_rerun branch November 26, 2025 14:14
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.

3 participants