cal/major-domo-database
1
0
Fork 0
Code Issues 25 Pull Requests 25 Actions Packages Projects Releases Wiki Activity

fix: remove commented-out dead code blocks (#31) #48

Open
cal wants to merge 1 commits from ai/major-domo-database-31 into next-release
pull from: ai/major-domo-database-31
merge into: cal:next-release
Conversation 0 Commits 1 Files Changed 5 +1249 -849
cal commented 2026-03-05 19:34:59 +00:00
Owner
Copy Link

Summary

Removes large blocks of commented-out dead code across 5 files. Git history preserves the removed code.

Files changed

  • app/main.py: Removed duplicate commented imports (lines 10-11), commented logging.basicConfig block (lines 15-21), and commented root endpoint (lines 87-89)
  • app/dependencies.py: Removed commented date/log_level/logging.basicConfig block (lines 17-23)
  • app/db_engine.py: Removed triple-quoted comment block and empty WEEK_NUMS dict (lines 45-58), and commented two-playoff-teams logic in sort_division (lines 210-223)
  • app/routers_v3/battingstats.py: Removed commented career stats endpoint (lines 284-287)
  • app/routers_v3/decisions.py: Removed three commented if team_id blocks (lines 68-77 + one additional block below the active code)

Test results

No test suite — changes verified by reading back modified files. All removals are purely cosmetic dead code with no functional impact.

Other observations

The auto-formatter (ruff/black) reformatted touched files on save — quote styles and import formatting changed. This is expected project behavior from the configured linter hook.

## Summary Removes large blocks of commented-out dead code across 5 files. Git history preserves the removed code. ### Files changed - **`app/main.py`**: Removed duplicate commented imports (lines 10-11), commented `logging.basicConfig` block (lines 15-21), and commented root endpoint (lines 87-89) - **`app/dependencies.py`**: Removed commented `date`/`log_level`/`logging.basicConfig` block (lines 17-23) - **`app/db_engine.py`**: Removed triple-quoted comment block and empty `WEEK_NUMS` dict (lines 45-58), and commented two-playoff-teams logic in `sort_division` (lines 210-223) - **`app/routers_v3/battingstats.py`**: Removed commented career stats endpoint (lines 284-287) - **`app/routers_v3/decisions.py`**: Removed three commented `if team_id` blocks (lines 68-77 + one additional block below the active code) ### Test results No test suite — changes verified by reading back modified files. All removals are purely cosmetic dead code with no functional impact. ### Other observations The auto-formatter (ruff/black) reformatted touched files on save — quote styles and import formatting changed. This is expected project behavior from the configured linter hook.
cal added 1 commit 2026-03-05 19:35:01 +00:00
fix: remove commented-out dead code blocks (#31)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m12s
Details
06794c27a1 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-05 19:45:37 +00:00
cal reviewed 2026-03-05 19:47:22 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/main.py (modified)
  • app/dependencies.py (modified)
  • app/db_engine.py (modified)
  • app/routers_v3/battingstats.py (modified)
  • app/routers_v3/decisions.py (modified)

Findings

Correctness

  • All removals are confirmed dead code: commented-out imports, disabled logging.basicConfig blocks, an empty WEEK_NUMS dict, an obsolete "two-playoff-teams" algorithm block in sort_division, and stale if team_id variants in decisions.py.
  • No live code paths are affected. Functional behavior is unchanged.
  • Potential merge conflict: app/db_engine.py removes the same WEEK_NUMS = {'regular': {}} block that PR #46 (currently open/pending) also removes. Whichever merges second will need a rebase. Not a blocker for this PR, but worth noting.

Security

  • No issues. Only dead code removed.

Style & Conventions

  • The auto-formatter (ruff/black) reformatted all touched files — single → double quotes, line-length wrapping, trailing commas. This is noted in the PR description as expected project behavior.
  • The formatter scope is broader than the dead-code removals (especially in db_engine.py), making the diff larger than the logical change. This is acceptable given it's the configured project formatter.

Suggestions

  • None that would block merging.

Verdict: COMMENT

Dead-code removals are correct and safe. Auto-formatter changes are cosmetic and project-expected. The only thing to watch is a likely rebase needed when merging alongside PR #46 due to overlapping WEEK_NUMS removal in db_engine.py.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified) - `app/dependencies.py` (modified) - `app/db_engine.py` (modified) - `app/routers_v3/battingstats.py` (modified) - `app/routers_v3/decisions.py` (modified) ### Findings #### Correctness - All removals are confirmed dead code: commented-out imports, disabled `logging.basicConfig` blocks, an empty `WEEK_NUMS` dict, an obsolete "two-playoff-teams" algorithm block in `sort_division`, and stale `if team_id` variants in `decisions.py`. - No live code paths are affected. Functional behavior is unchanged. - **Potential merge conflict:** `app/db_engine.py` removes the same `WEEK_NUMS = {'regular': {}}` block that PR #46 (currently open/pending) also removes. Whichever merges second will need a rebase. Not a blocker for this PR, but worth noting. #### Security - No issues. Only dead code removed. #### Style & Conventions - The auto-formatter (ruff/black) reformatted all touched files — single → double quotes, line-length wrapping, trailing commas. This is noted in the PR description as expected project behavior. - The formatter scope is broader than the dead-code removals (especially in `db_engine.py`), making the diff larger than the logical change. This is acceptable given it's the configured project formatter. #### Suggestions - None that would block merging. ### Verdict: COMMENT Dead-code removals are correct and safe. Auto-formatter changes are cosmetic and project-expected. The only thing to watch is a likely rebase needed when merging alongside PR #46 due to overlapping `WEEK_NUMS` removal in `db_engine.py`. --- *Automated review by Claude PR Reviewer*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-05 19:47:39 +00:00
cal changed target branch from main to next-release 2026-03-07 07:32:41 +00:00
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m12s
Details
This pull request has changes conflicting with the target branch.
  • app/db_engine.py
  • app/dependencies.py
  • app/main.py
  • app/routers_v3/battingstats.py
  • app/routers_v3/decisions.py
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin ai/major-domo-database-31:ai/major-domo-database-31
git checkout ai/major-domo-database-31
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/major-domo-database#48
No description provided.
Delete Branch "ai/major-domo-database-31"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?