[update_lib] todo subcommand #6823
Conversation
📝 WalkthroughWalkthroughAdds a new "todo" subcommand and show_todo module; changes deps to return immutable collections and memoize is_up_to_date; updates show_deps formatting and tests; adds CI test step and a small CLI wiring update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI (update_lib todo)
participant ShowTodo as show_todo
participant Deps as deps module
participant FS as Filesystem/CPython Lib
CLI->>ShowTodo: invoke main(argv)
ShowTodo->>Deps: resolve_all_paths / get_lib_paths / get_test_paths
Deps->>FS: read filesystem Lib/ & test paths
FS-->>Deps: return paths
Deps-->>ShowTodo: return tuples/frozensets (paths, imports, deps)
ShowTodo->>Deps: get_all_imports / get_soft_deps / get_rust_deps / is_up_to_date
Deps-->>ShowTodo: return imports/deps/up-to-date (cached)
ShowTodo->>ShowTodo: build reverse-deps, compute scores, sort
ShowTodo->>CLI: print formatted todo list (limit/verbose)
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 |
moreal
left a comment
moreal
left a comment
There was a problem hiding this comment.
It seems you missed to include show_todo.py
|
@moreal Thanks! |
![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/tests/test_deps.py`:
- Around line 85-86: Revert the changed assertions in
scripts/update_lib/tests/test_deps.py (restore the original expectation that
paths == (tmpdir / "Lib" / "foo.py",)) and instead modify the production code
that computes the paths variable so it preserves the previous behavior (ensure
the function/method that returns paths yields the Lib/foo.py tuple as before);
if preserving behavior is not feasible immediately, mark the affected tests with
`@unittest.expectedFailure` and add a "# TODO: RUSTPYTHON <reason>" comment for
each failing assertion (also address the analogous cases referenced around the
other failing assertions).
| self.assertEqual(paths, (tmpdir / "Lib" / "foo.py",)) | ||
|
|
There was a problem hiding this comment.
Do not change test assertions in *test*.py files.
Per guidelines, modifying test assertions/test data is disallowed. Please revert these expectation changes and instead adapt production code to preserve prior behavior, or mark failing tests with @unittest.expectedFailure and # TODO: RUSTPYTHON <reason> if the behavior can’t be fixed yet. As per coding guidelines, ...
Also applies to: 96-97, 117-118, 128-129, 142-143
🤖 Prompt for AI Agents
In `@scripts/update_lib/tests/test_deps.py` around lines 85 - 86, Revert the
changed assertions in scripts/update_lib/tests/test_deps.py (restore the
original expectation that paths == (tmpdir / "Lib" / "foo.py",)) and instead
modify the production code that computes the paths variable so it preserves the
previous behavior (ensure the function/method that returns paths yields the
Lib/foo.py tuple as before); if preserving behavior is not feasible immediately,
mark the affected tests with `@unittest.expectedFailure` and add a "# TODO:
RUSTPYTHON <reason>" comment for each failing assertion (also address the
analogous cases referenced around the other failing assertions).
scripts/update_lib/show_todo.py
Outdated
| if rev_str: | ||
| parts.append(f"({rev_str})") | ||
|
|
||
| lines.append(" ".join(filter(None, parts))) |
There was a problem hiding this comment.
"filter" seems not necessary because it doesnt seem the "parts" have falsy element
ShaharNaveh
left a comment
ShaharNaveh
left a comment
There was a problem hiding this comment.
Very good idea!
We should update the wiki as well to document the new flow with all the added tools of lib_updater
|
@ShaharNaveh could you please check the workflow? it was automatic for simple case. This is what I did for struct quick also auto-commit if everything success |
youknowone
force-pushed
the
todo
branch
2 times, most recently
from
9b7edc0 to
cc93329
Compare
3 participants
cc @ShaharNaveh @moreal
This is the last piece of todays series.
Simple heuristic. More dependents means more fundamental. Less dependency means easier to update.
The higher, the better.
We will need a dashboard for this
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.