Skip to content

[NO QA] Use postCodeReviewResults.sh from claude-review-toolkit#91614

Closed
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:slice2-adopt-postCodeReviewResults
Closed

[NO QA] Use postCodeReviewResults.sh from claude-review-toolkit#91614
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:slice2-adopt-postCodeReviewResults

Conversation

@kacper-mikolajczak
Copy link
Copy Markdown
Contributor

Explanation of Change

Adopts the new postCodeReviewResults.sh helper added in Expensify/GitHub-Actions#66. Replaces the inline jq + while-loop in the Post code review results step with a single script call. Bumps the claude-review-toolkit pin to the SHA that introduces the script.

No steady-state behaviour change: 0 violations -> +1 reaction; N violations -> N inline comments via createInlineComment.sh (failures per comment swallowed with || true); empty STRUCTURED_OUTPUT -> ::error:: + exit 1.

This is part of slice 2 of the centralisation effort. The toolkit pin currently points at the SHA on the toolkit PR branch and will be re-stamped to the merged-main SHA once Expensify/GitHub-Actions#66 lands.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/635397
PROPOSAL:

Tests

N/A - CI infrastructure with no user-facing behaviour change. Validation happens via the workflow itself running on this PR (will only fully validate once the toolkit SHA is re-stamped post-merge of Expensify/GitHub-Actions#66).

  1. Verify that no errors appear in the JS console

Offline tests

N/A - CI infrastructure change; doesn't affect runtime offline behaviour.

QA Steps

@Julesssss and @kacper-mikolajczak will check out some AI Reviewer workflow runs on real PRs after merge.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • 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 added steps for local testing in the `Tests` section
    • I added steps for the expected offline behavior in the `Offline steps` section
    • I added steps for Staging and/or Production testing in the `QA steps` section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • 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 (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly

Screenshots/Videos

Android: Native N/A - CI infra
Android: mWeb Chrome N/A - CI infra
iOS: Native N/A - CI infra
iOS: mWeb Safari N/A - CI infra
MacOS: Chrome / Safari N/A - CI infra

cc @Julesssss

Replaces the inline jq+while-loop in the 'Post code review results'
step with a single postCodeReviewResults.sh call. Behaviour identical
to today: 0 violations leaves a +1 reaction, otherwise N inline
comments are posted via createInlineComment.sh.

Bumps the claude-review-toolkit pin to the SHA that introduces the
new script.
@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