Skip to content

feat: add about subcommand#50

Merged
crossjam merged 7 commits into
mainfrom
feat/issue-14-about-subcommand
May 29, 2026
Merged

feat: add about subcommand#50
crossjam merged 7 commits into
mainfrom
feat/issue-14-about-subcommand

Conversation

@crossjam
Copy link
Copy Markdown
Owner

@crossjam crossjam commented May 29, 2026

Summary

  • Add a new scrobbledb about command with project metadata and default path information
  • Refresh locked dependencies to current secure releases
  • Fix a couple of follow-up issues in CLI error reporting and type annotations

Validation

  • uv run poe qa
  • uv run poe lint
  • uv run poe type
  • uv run pytest tests/test_cli.py tests/test_docs_generation.py -q

Notes

This draft PR includes the full set of commits we worked on for issue #14.

@crossjam crossjam self-assigned this May 29, 2026
@crossjam
Copy link
Copy Markdown
Owner Author

Code Review Summary

Verdict: Changes requested before merge

The new scrobbledb about command is useful and the implementation is close, but I found one blocking issue that should be fixed before merging.

Blocking

  • scrobbledb about has filesystem side effects even though it is an informational command.
    • In src/scrobbledb/cli.py, about() calls get_default_db_path() and get_default_auth_path().
    • Those helpers currently call data_dir.mkdir(parents=True, exist_ok=True), so running scrobbledb about creates the application data directory.
    • I verified this by running the command with HOME pointed at a fresh temporary directory; the app directory was created as a side effect.

Why this matters:

  • about reads like a safe introspection/help command.
  • Creating user state on disk just to display default paths is surprising behavior.

Suggested fix:

  • Make the path helpers pure for read/display use, or split them into:
    • a non-mutating path computation helper for commands like about
    • a separate ensure_* helper for commands that actually need to write files/directories
  • Add a regression test asserting that scrobbledb about does not create the data directory.

Non-blocking

  • The hardcoded summary/repository/authors strings in about() could drift from package metadata over time. Pulling these from package metadata would make the command easier to maintain.
  • This PR also includes some unrelated churn (uv.lock, domain_format.py type-ignore changes, and the import error guard), which makes the feature review a bit noisier.

Validation

  • uv run poe test:quick passes locally
  • GitHub checks on the PR are passing
  • scrobbledb about otherwise prints the expected information

Once the side effect is removed, I’d be comfortable merging this.

@crossjam
Copy link
Copy Markdown
Owner Author

Follow-up: I addressed the review feedback in commit 9509323.

What changed:

  • scrobbledb about no longer creates the app data directory as a side effect.
    • The default-path helpers are now pure path computations.
    • Directory creation was moved to explicit write-path call sites (auth, default-log-config setup, and default-db setup for ingest).
  • about now reads summary/repository/authors from package metadata instead of hardcoded strings.
  • Added regression coverage for:
    • no filesystem mutation from scrobbledb about
    • metadata-driven output

Validation:

  • uv run pytest tests/test_cli.py -q
  • uv run poe test:quick

Both are passing locally.

@crossjam crossjam marked this pull request as ready for review May 29, 2026 23:13
@crossjam
Copy link
Copy Markdown
Owner Author

Final review pass complete.

Current status:

  • scrobbledb about is side-effect free
  • package metadata is sourced dynamically
  • regression coverage was added for the about command behavior
  • formatter typing cleanup landed, removing the ty: ignore[call-non-callable] comments in domain_format.py
  • local uv run poe qa passes
  • GitHub QA checks are green across the matrix

From my side, this looks ready to merge.

@crossjam
Copy link
Copy Markdown
Owner Author

LGTM! Fire in the disco!

@crossjam crossjam merged commit 4290cb3 into main May 29, 2026
6 checks passed
@crossjam crossjam deleted the feat/issue-14-about-subcommand branch May 29, 2026 23:40
@crossjam crossjam mentioned this pull request May 29, 2026
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