Conversation
Refactor for naming consistency and repository cleanup: - Rename many functions/variables to snake_case (e.g. submitAPI→submit_api, Get_MwClient_Site→get_mwclient_site, Claim_API_str→claim_api_str, Labels_API→labels_api, Delete_claim→delete_claim, ViewsData→views_data, Cat_Depth→cat_depth, etc.) and update all imports/call sites. - Rename td_core/fix_user_pages/del.py → del_records.py and update docs/toolforge job entries. - Remove legacy wprefs.bots.replace_except module and its tests. - Extend pyproject.toml excludes/ignore (skip src1 and common cache/build paths). Purpose: improve code consistency, maintainability, and tool scanning behavior.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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 |
MrIbrahem
left a comment
There was a problem hiding this comment.
Review summary
Large mechanical refactor: function/variable names normalized to snake_case, one file renamed (del.py → del_records.py), an unused module + its tests deleted, and tooling excludes added. Rewrites are consistent and ruff check src/ is clean except for the module-name issue below. However, the rename is incomplete for one module, which breaks the documented run command and the docs, and there are a few stale/committed environment-specific config items.
Verdict: 💬 Comment (forced — you're the PR author; GitHub does not allow APPROVE/REQUEST_CHANGES on your own PR). Underlying assessment: 🔴 blocking issue found (see inline).
Title
Title is accurate and clear — "Normalize names to snake_case and clean up repository" correctly describes both halves of the change. No change needed.
Highlights
- Function/parameter renames are applied consistently; no leftover PascalCase call sites in
src/(verified by grep +ruff). src/newapi/__init__.pyimport reshuffle resolves correctly (function_timervia theutils/package,AskBot/HandleErrorsviaapi_utils,MainPageviapages).- Deletion of the unused local
replace_exceptmodule and its two test files is consistent — those tests only imported the deleted module, so no orphaned tests remain.
Issues found
- 🔴 Blocking: 1 — incomplete rename of
Medicine_articles.py(see inline). - 🟡 Suggestions: 3 — stale
del_records.pydocstring command, staleCLAUDE.mdreference to a deleted module, and machine-specific path exclusions committed topyproject.toml.
Cross-cutting notes
- Stale command in
del_records.py: that file was renamed fromdel.py→del_records.py, but its docstring still sayscore8/pwb.py td_core/fix_user_pages/del. Update it totd_core/fix_user_pages/del_records. (Not left as an inline comment because a 100%-similarity rename has no diff hunk to anchor to.) src1/(a duplicate ofsrc/) is committed to the repo and this PR adds it to the tooling excludes. That duplication is pre-existing tech debt worth addressing separately; the added exclusions here are consistent with that intent, just flagging the broader smell.- The big new
ignore = [...]block inpyproject.tomllargely duplicates the existingexcludelist — consider collapsing it.
No description provided.