update_lib todo also shows test todo #6859
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds test-awareness to Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin todo |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/update_lib/show_todo.py`:
- Around line 253-256: The docstring and implementation disagree on the scoring
values; update the logic that assigns the variable score in this module so it
matches the docstring: set score = 0 when the corresponding lib is up-to-date,
set score = 1 when the corresponding lib exists but is NOT up-to-date, and set
score = -1 when there is no corresponding lib (ensure any places that check or
return score use these values consistently, e.g., the score assignment sites and
any functions that consume score).
🧹 Nitpick comments (1)
scripts/update_lib/show_todo.py (1)
408-419: Redundantis_up_to_datecalls double the file comparison work.
compute_todo_listinternally callsis_up_to_datefor each module (see line 45 in existing code). Then lines 413-414 iterate over all modules callingis_up_to_dateagain. This duplicates potentially expensive file comparison operations.Consider extracting
lib_statusfromlib_todoresults instead:♻️ Proposed refactor
# Compute lib todo lib_todo = compute_todo_list(cpython_prefix, lib_prefix, include_done) - # Build lib status map for test scoring - lib_status = {} - for name in get_all_modules(cpython_prefix): - lib_status[name] = is_up_to_date(name, cpython_prefix, lib_prefix) + # Build lib status map from lib_todo results to avoid redundant is_up_to_date calls + # Note: compute_todo_list already called is_up_to_date for each module + lib_status = {item["name"]: item["up_to_date"] for item in lib_todo} + # If include_done=False, lib_todo only has non-up-to-date items; need full status + if not include_done: + from update_lib.deps import is_up_to_date + for name in get_all_modules(cpython_prefix): + if name not in lib_status: + lib_status[name] = True # Not in lib_todo means it's up-to-dateAlternatively, refactor
compute_todo_listto return the full status map alongside the filtered results.
| Scoring: | ||
| - If corresponding lib is up-to-date: score = 0 (ready) | ||
| - If corresponding lib is NOT up-to-date: score = 1 (wait for lib) | ||
| - If no corresponding lib: score = -1 (independent) |
There was a problem hiding this comment.
Docstring does not match implementation.
The docstring states scores of 0, 1, and -1, but the implementation uses 0, 2, and 1:
score = 0: lib is up-to-date (matches)score = 2: lib NOT up-to-date (docstring says1)score = 1: no corresponding lib (docstring says-1)
📝 Proposed fix
"""Compute prioritized list of tests to update.
Scoring:
- - If corresponding lib is up-to-date: score = 0 (ready)
- - If corresponding lib is NOT up-to-date: score = 1 (wait for lib)
- - If no corresponding lib: score = -1 (independent)
+ - If corresponding lib is up-to-date: score = 0 (ready to update)
+ - If no corresponding lib: score = 1 (independent test)
+ - If corresponding lib is NOT up-to-date: score = 2 (wait for lib first)
Returns:
List of dicts with test info, sorted by priority
"""📝 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.
| Scoring: | |
| - If corresponding lib is up-to-date: score = 0 (ready) | |
| - If corresponding lib is NOT up-to-date: score = 1 (wait for lib) | |
| - If no corresponding lib: score = -1 (independent) | |
| Scoring: | |
| - If corresponding lib is up-to-date: score = 0 (ready to update) | |
| - If no corresponding lib: score = 1 (independent test) | |
| - If corresponding lib is NOT up-to-date: score = 2 (wait for lib first) |
🤖 Prompt for AI Agents
In `@scripts/update_lib/show_todo.py` around lines 253 - 256, The docstring and
implementation disagree on the scoring values; update the logic that assigns the
variable score in this module so it matches the docstring: set score = 0 when
the corresponding lib is up-to-date, set score = 1 when the corresponding lib
exists but is NOT up-to-date, and set score = -1 when there is no corresponding
lib (ensure any places that check or return score use these values consistently,
e.g., the score assignment sites and any functions that consume score).
1 participant
also tracking untracked files considered in #6775
cc @terryluan12
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.