📦 [Consolidated] PR #59, #67 の github.ts 改善統合・レビュー指摘対応#66
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Extract duplicated rate limit reset timestamp calculation into a single helper function. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、GitHubのpublic eventsを取得する際のパフォーマンスを大幅に改善することを目的としています。以前はページごとに直列にリクエストを送信していたため、ネットワークの遅延が累積していましたが、今回の変更により、最大3ページ分のリクエストを並列で実行することで、既存のビジネスロジックを維持しつつ、I/O処理の効率化を実現しました。これにより、モック環境で約3分の1に高速化され、実際のネットワーク環境ではさらに大きな遅延削減効果が期待されます。 Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
# Conflicts: # src/lib/github.ts
…uppressed errors Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
…9680 # Conflicts: # .github/copilot-instructions.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/github.ts`:
- Around line 640-641: The linter flags the promises.forEach callback because
p.catch(...) returns a Promise which forEach ignores; fix by explicitly
signaling intent or iterating properly: either prefix the callback call with the
void operator (change the callback passed to promises.forEach in the code that
currently references promises.forEach((p) => p.catch(...)) to use void
p.catch(...)) or replace the forEach with an explicit for-of loop over the
promises array and call p.catch(...) inside it; update the occurrence that uses
promises.forEach in this file (the suppressed event fetch error line)
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 14fc50a0-7ccd-4f74-92a8-6c14eb33fe55
📒 Files selected for processing (1)
src/lib/github.ts
| // Suppress unhandled promise rejections for subsequent pages if we break early or throw | ||
| promises.forEach((p) => p.catch((e) => console.error("Suppressed event fetch error:", e))); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
forEach コールバックから値を返しているため、リンターエラーが発生しています。
p.catch() は Promise を返しますが、forEach はコールバックの戻り値を無視します。Biome がこれを疑わしいパターンとしてフラグしています。void 演算子を使用して意図を明確にするか、for-of ループに変更することで解決できます。
♻️ 修正案
オプション 1: void 演算子を使用
- promises.forEach((p) => p.catch((e) => console.error("Suppressed event fetch error:", e)));
+ promises.forEach((p) => void p.catch((e) => console.error("Suppressed event fetch error:", e)));オプション 2: for-of ループを使用
- promises.forEach((p) => p.catch((e) => console.error("Suppressed event fetch error:", e)));
+ for (const p of promises) {
+ p.catch((e) => console.error("Suppressed event fetch error:", e));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Suppress unhandled promise rejections for subsequent pages if we break early or throw | |
| promises.forEach((p) => p.catch((e) => console.error("Suppressed event fetch error:", e))); | |
| // Suppress unhandled promise rejections for subsequent pages if we break early or throw | |
| promises.forEach((p) => void p.catch((e) => console.error("Suppressed event fetch error:", e))); |
🧰 Tools
🪛 Biome (2.4.6)
[error] 641-641: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/github.ts` around lines 640 - 641, The linter flags the
promises.forEach callback because p.catch(...) returns a Promise which forEach
ignores; fix by explicitly signaling intent or iterating properly: either prefix
the callback call with the void operator (change the callback passed to
promises.forEach in the code that currently references promises.forEach((p) =>
p.catch(...)) to use void p.catch(...)) or replace the forEach with an explicit
for-of loop over the promises array and call p.catch(...) inside it; update the
occurrence that uses promises.forEach in this file (the suppressed event fetch
error line) accordingly.
概要
このPRは、
src/lib/github.tsに対する関連変更である以下2個のPRを一つのブランチにて統合(マージ)し、競合解消とレビューコメント対応を行ったうえで、GitHub API 周辺の改善を一本のPRにまとめるためのものです。統合元のPR(これらは本作業に伴いクローズ対象です):
🧹 [Code Health] Refactor graphql utility to use handleResponse🧹 Extract getRateLimitReset helper function in github.ts対応した修正点 (レビュー指摘への対応)
src/lib/github.tsの競合解消fetchActivity並列取得改善をベースにしつつ、PR 🧹 [Code Health] Refactor graphql utility to use handleResponse #59 のgraphql()におけるhandleResponse()再利用と、PR 🧹 Extract getRateLimitReset helper function in github.ts #67 のgetRateLimitReset()ヘルパー抽出を同時に取り込みました。src/lib/github.ts(PR 📦 [Consolidated] PR #59, #67 の github.ts 改善統合・レビュー指摘対応 #66 指摘対応)console.errorで抑止したエラーを記録する形に変更し、デバッグ可能性を残しました。既存動作を崩さない形での整理
確認事項
src/lib/github.tsに集中した差分を 📦 [Consolidated] PR #59, #67 の github.ts 改善統合・レビュー指摘対応 #66 に集約したうえで、追加 push 後の review 状態と GitHub Actions の CI 状態をまとめて確認します。