Skip to content

refactor: move list_tools to utils and remove rename_readings script#1042

Merged
fshowalter merged 1 commit intomainfrom
draft
Mar 22, 2026
Merged

refactor: move list_tools to utils and remove rename_readings script#1042
fshowalter merged 1 commit intomainfrom
draft

Conversation

@fshowalter
Copy link
Copy Markdown
Owner

No description provided.

@fshowalter fshowalter enabled auto-merge (squash) March 22, 2026 14:37
@claude
Copy link
Copy Markdown

claude bot commented Mar 22, 2026

Claude finished @fshowalter's task in 35s —— View job


PR Review

  • Gather context and review changed files
  • Analyze code quality and correctness
  • Post review feedback

This is a clean, straightforward refactor. Here's my assessment:

What changed:

  • Deleted booklog/exports/list_tools.py (26 lines) — moved to booklog/utils/list_tools.py
  • Updated imports in reading_log.py and stats.py to use the new location
  • Deleted scripts/rename_readings.py (124 lines) — a one-off migration script

Code quality: ✅

The consolidation makes sense. list_tools utilities (group_list_by_key, list_to_dict_by_key) are general-purpose and belong in utils/ rather than exports/. No callers of the old location remain — the import migration is complete and correct.

One minor observation: the deleted booklog/exports/list_tools.py contained list_to_dict_by_key in addition to group_list_by_key, but list_to_dict_by_key was not migrated to booklog/utils/list_tools.py. A quick grep confirms it has no remaining callers, so the removal is intentional and safe.

No issues found. This PR is a straightforward cleanup — net -152 lines with no regressions.

@fshowalter fshowalter merged commit d4b3094 into main Mar 22, 2026
6 checks passed
@fshowalter fshowalter deleted the draft branch March 22, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant