[No QA] Paralellize typecheck - #99743
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@abzokhattab Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Looks like it broke tests .. weird .. is it already broken on main? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b6948d6ee
ℹ️ 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".
2b6948d to
e08bd3e
Compare
ce677a5 to
8e0df3c
Compare
Yeah broken on main. Someone put current date in the test and it worked yesterday, but not today xd |
|
@MelvinBot review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb Safari |
Review: no blocking issuesThe change is correct. One suggestion worth applying, plus a calibration note on the perf claim. Suggestion: don't withhold all output until the last project finishes
Awaiting the already-started promises in order gives identical concurrency but starts printing as soon as project 1 lands, still grouped and still in a stable order: const running = projects.map(async (project) => {
// ...unchanged...
return {tsconfig, result};
});
const failed: string[] = [];
for (const promise of running) {
const {tsconfig, result} = await promise;
console.log(`\nType checking ${tsconfig}...`);
// ...unchanged...
}The CI gain is closer to ~10%, not 30%CI runs on
Still a real win, just smaller than the local numbers in the description — worth a tweak to the PR body so nobody is surprised later. The failing check is pre-existing
Also: the P1 inline comment from Codex on What I checked and found clean
Nit, very low severity: if |
abzokhattab
left a comment
There was a problem hiding this comment.
LGTM .. no issues found
Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.70-0 🚀
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.72-0 🚀
|

Explanation of Change
Run the four typecheck projects concurrently instead of sequentially in
scripts/typecheck.ts, cutting fulltypecheckwall time by ~30% (16.4s → 11.4s cold, 1.45s → 1.05s warm) with per-project output grouping preserved.Details in proposal linked below.
[No QA] because it only affects dev tooling
Fixed Issues
$ #99889
PROPOSAL:
https://expensify.slack.com/archives/C08CZDJFJ77/p1788162565862549
Tests
npm run typecheckin the consoleOffline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
N/A