Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a2c49424d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
.github/scripts/verifyDocs.sh
Outdated
| declare -r NC='\033[0m' | ||
|
|
||
| printf '\nRebuilding API docs...\n' | ||
| npm run build:docs |
There was a problem hiding this comment.
Fail validation when docs generation command errors
The result of npm run build:docs is not checked, so the script can still exit successfully if docs generation fails but API.md/API-INTERNAL.md remain unchanged. In a PR that introduces a TypeScript or JSDoc generation error, this check would incorrectly report "API docs are up to date" and allow a broken docs pipeline to merge. The script should stop immediately on build failure (for example via set -e or an explicit exit-status check).
Useful? React with 👍 / 👎.
.github/workflows/validateDocs.yml
Outdated
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: ['lib/**', 'buildDocs.ts', 'API.md', 'API-INTERNAL.md'] |
There was a problem hiding this comment.
Trigger docs validation when validator files change
The pull_request.paths filter omits the validator itself (.github/scripts/verifyDocs.sh) and this workflow file, so PRs that only modify docs-validation logic will skip this job entirely. That allows regressions in the validation script/workflow to merge without any execution of the check. Include the validator/workflow paths (or remove the restrictive filter) so changes to the validation mechanism are tested.
Useful? React with 👍 / 👎.
Details
Add check to verify API docs
Related Issues
Expensify/App#80097
Automated Tests
N/A
Manual Tests
N/A
Author Checklist
### Related Issuessection aboveTestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari