Skip to content

Split docs and code review into separate jobs in claude-review.yml#91617

Closed
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:slice2-untangle-docs-code-review
Closed

Split docs and code review into separate jobs in claude-review.yml#91617
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:slice2-untangle-docs-code-review

Conversation

@kacper-mikolajczak
Copy link
Copy Markdown
Contributor

Explanation of Change

Untangles the docs and code review flows in .github/workflows/claude-review.yml by splitting the existing single review job into two sibling jobs (review-code and review-docs).

Today the workflow runs both claude-code-action invocations sequentially inside one job, gated by a single dorny/paths-filter step that emits two outputs. That makes the code-review path diverge from the shape used by Expensify/Auth and Expensify/Web-Expensify, which each run a single claude-code-action per job. This PR aligns the code-review path with that shape so the three repos can converge on a shared claude-review workflow in a follow-up slice.

What changed:

  • review job split into review-code and review-docs. Both depend on the existing validate and checkCPlusApproval gate jobs via needs:, so contributor authorisation is unchanged.
  • Each job owns its own dorny/paths-filter step (src/** for code, docs/**/*.md + docs/**/*.csv for docs), its own toolkit setup, its own eyes add/remove lifecycle, and its own claude-code-action call.
  • review-code keeps the Setup Node step because the code review prompt uses Bash(check-compiler.sh:*) from its allowedTools. review-docs does not need it and skips the setup.
  • Workflow-level name, permissions, on:, concurrency, and the validate / checkCPlusApproval jobs are untouched.

Net steady-state behaviour is identical: same triggers, same contributor gate, same path filtering, same prompts, same allowedTools, same toolkit SHA pin, same claude-code-action SHA pin, same post-processing of structured output, same eyes reactions on PRs that touch code and/or docs.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/635397
PROPOSAL: N/A (infra refactor, no proposal required)

Related Issues

Tests

This change only touches a GitHub Actions workflow, so the verification is observational on the workflow runs themselves:

  1. Open a draft PR against main, mark it ready for review.
  2. Verify both review-code and review-docs jobs are visible in the Actions tab under the PR Reviews with Claude Code workflow.
  3. Verify the validate job runs once and both review jobs depend on it.
  4. On a PR that only touches src/**, verify review-code posts comments / +1 reaction and review-docs short-circuits (path filter false, no eyes, no claude-code-action invocation).
  5. On a PR that only touches docs/**/*.md or docs/**/*.csv, verify the reverse.
  6. On a PR that touches both, verify both jobs run and each posts its own results independently.
  7. On a draft PR or a PR with Revert in the title, verify neither review job runs.
  8. Verify a pull_request_review submitted event from a Contributor+ reviewer still triggers both jobs as before.

Offline tests

N/A - GitHub Actions workflow change, no runtime client behaviour.

QA Steps

Same as Tests above; QA can observe the workflow runs on real PRs once merged.

PR Author Checklist

CI-only change (single workflow file). The platform-test and runtime checklist items below do not apply to this PR.

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors
  • I followed proper code patterns
  • I tested other components that can be impacted by my changes
  • I verified all code is DRY
  • I added unit tests for any new feature or bug fix in this PR

Untangles the two review flows so the code-review path matches the
single-claude-code-action shape used by Expensify/Auth and
Expensify/Web-Expensify. The validate and checkCPlusApproval gate jobs
are unchanged; both new jobs depend on them and share the existing
concurrency group.
@kacper-mikolajczak
Copy link
Copy Markdown
Contributor Author

Superseded by #91689 (consolidated slice 2 work into one PR per repo).

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.

1 participant