Skip to content

[No QA] Automate PR checklist batch 2: callback naming, prop docs, file headers#95545

Closed
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:kacper/92599-automate-checklist-batch-2
Closed

[No QA] Automate PR checklist batch 2: callback naming, prop docs, file headers#95545
kacper-mikolajczak wants to merge 1 commit into
Expensify:mainfrom
kacper-mikolajczak:kacper/92599-automate-checklist-batch-2

Conversation

@kacper-mikolajczak

Copy link
Copy Markdown
Contributor

Explanation of Change

Phase 2 (batch 2) of automating the deterministic PR-checklist items behind the code-inline-reviewer, following the merged Phase 1 (#92926). Converts three more checklist items into auto-discovered reviewer rules and trims the checklists accordingly.

New reviewer rules (.claude/skills/coding-standards/rules/, also registered in SKILL.md):

Rule Enforces Replaces checklist item
CONSISTENCY-12 Callbacks named for the action, not the event (toggleReport not onIconClick) author + reviewer "callback methods ... named for what the method does"
CONSISTENCY-13 Component props carry a /** */ block comment (presence check; complements CONSISTENCY-10 which owns style + mixed cases, so no double-flag) reviewer "each prop has a /** comment above it */"
CONSISTENCY-14 Non-trivial new files start with a header description comment author + reviewer "file has a description ... at the top"

Checklist trims

  • Removed the 3 converted items above from PULL_REQUEST_TEMPLATE.md and REVIEWER_CHECKLIST.md.
  • Removed reviewer lines already dead-covered by existing rules: "file is named correctly" (CONSISTENCY-9), "only data ... necessary for rendering" (CLEAN-REACT-5), and the three Class Component this-binding / render-method items (moot under CLEAN-REACT-8, which bans class components).

⚠️ Merge ordering

Trimming REVIEWER_CHECKLIST.md shortens it, which re-triggers the exact regression fixed by #94161 (the reviewer-checklist action caps completed items at count + 2). #94161 must merge before this PR or in-flight reviewer checklists written against the longer template will fail.

Fixed Issues

$ #92599

Tests

Deterministic rule/markdown changes; no runtime surface. Verified by inspection that each removed checklist line maps to a rule that enforces it (table above).

  • Verify that no errors appear in the JS console

QA Steps

[No QA]

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above

…headers

Add reviewer rules CONSISTENCY-12/13/14 and trim the matching author +
reviewer checklist items, plus remove reviewer lines now dead-covered by
existing rules (CONSISTENCY-9, CLEAN-REACT-5, CLEAN-REACT-8).

Issue: Expensify#92599
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


Kacper Mikołajczak seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@kacper-mikolajczak

Copy link
Copy Markdown
Contributor Author

Reopening from the Callstack internal fork (callstack-internal/Expensify-App) - superseded by #95546.

@kacper-mikolajczak
kacper-mikolajczak deleted the kacper/92599-automate-checklist-batch-2 branch July 7, 2026 22:47
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