Convert repo IO and processing to async/await#260
Conversation
Replace blocking fs/path calls with node:fs/promises and node:path and convert package retrieval, repo discovery, and git reflog processing to async/await to avoid synchronous I/O and improve robustness and error propagation. Improve error construction to preserve the original error as the cause. Refactor git output parsing to filter earlier and collect per-repo errors without failing the whole run. Update tests to use async patterns and the platform path separator. Changed files: - src/common/repos.js - src/grefplus/cmdline.js - src/grefplus/index.js - test/repos.spec.js
|
Warning Review limit reached
More reviews will be available in 9 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR converts filesystem access from synchronous Node ChangesAsync/await migration for repository and package utilities
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/grefplus/index.js`:
- Around line 60-79: The code collapses repo to basename(repo) and loses root
context; change the repo identifier used in lines.push and errors.push to
preserve full or root-relative path (e.g., stop using repoName from
basename(repo) and instead use repo or path.relative(root, repo) so duplicate
basenames remain distinguishable), update the occurrences where repoName is
assigned and where errors.push({ repo: basename(repo) ... }) to use the chosen
full/relative path, and ensure any logging or downstream consumers that expect
repoName are updated accordingly.
- Around line 120-123: The current code maps over repos to start one process per
repo and awaits Promise.all(promises), which can spawn unbounded git processes;
change this to a bounded concurrency approach by introducing a concurrency limit
(e.g., const CONCURRENCY = N) and use a worker/pool or a small promise-limiter
to only run up to CONCURRENCY concurrent calls to processRepo; specifically,
replace the direct repos.map(...) usage with a limited runner that enqueues
processRepo(repo, errors) tasks and schedules them with at most CONCURRENCY
active tasks (or use a lightweight helper like p-limit to wrap processRepo),
then await Promise.all on the array of limited promises so no more than the
configured number of git log processes run at once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5d2f8d0-01cd-494f-956f-059cca56c9bc
📒 Files selected for processing (4)
src/common/repos.jssrc/grefplus/cmdline.jssrc/grefplus/index.jstest/repos.spec.js
Convert CLI yargs checks to await the asynchronous version validator so repository engine checks can perform async I/O. Update unit tests to use async/await and adjust stubs to return/resolves values; add tests covering oldest/max/min selection logic. Remove slow/flaky git integration tests. Update the temp test to use a real temp base. Also bump dependency versions for simple-git and uuid. Changed files: - package.json - src/spawner/index.js - src/yarn/index.js - test/engine.spec.js - test/temp.spec.js - test/git-seq.spec.js (deleted) - test/git.spec.js (deleted)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Throttle repo processing by batching repos into concurrent
chunks of 8 to avoid spawning too many parallel tasks and to
reduce resource contention.
Strengthen error reporting by narrowing the errors param to an
array of {repo, error} and by recording the full repo path rather
than the basename when pushing errors.
Update tests to import node:fs and node:path and fix a typo in a
test description; minor cleanup of temp init/destroy calls.
Changed files:
- src/grefplus/index.js
- test/temp.spec.js
|
2 participants
Replace blocking fs/path calls with node:fs/promises and node:path and convert package retrieval, repo discovery, and git reflog processing to async/await to avoid synchronous I/O and improve robustness and error propagation.
Improve error construction to preserve the original error as the cause. Refactor git output parsing to filter earlier and collect per-repo errors without failing the whole run. Update tests to use async patterns and the platform path separator.
Changed files:
Summary by CodeRabbit
Refactor
Bug Fixes