Skip to content

Conversation

@Kludex
Copy link
Member

@Kludex Kludex commented Jan 27, 2026

No description provided.

- Simplify multi-line context managers to single lines where readable
- Use logger.exception() instead of logger.error() for exception logging
- Remove redundant Args sections from internal method docstrings
- Simplify _validate_content_type to a single return statement
- Use Field(default_factory=list) for mutable defaults in Pydantic models
- Convert Field descriptions to class-level docstrings

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Comment on lines +219 to +220
except Exception:
logger.exception(f"Session {http_transport.mcp_session_id} crashed")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep finding those.

@Kludex Kludex requested a review from maxisbey January 27, 2026 09:14
@claude
Copy link

claude bot commented Jan 27, 2026

Code review

Code Changes: No issues found. The refactoring correctly improves code style and follows Python best practices.

Process Issues: Two CLAUDE.md violations in PR metadata:

  1. Commit message contains Co-Authored-By trailer

    • Violates CLAUDE.md: "NEVER ever mention a co-authored-by or similar aspects. In particular, never mention the tool used to create the commit message or PR."
    • Found in commit: Co-Authored-By: Claude Opus 4.5 <[email protected]>
  2. Empty PR description

    • Violates CLAUDE.md: "Create a detailed message of what changed. Focus on the high level description of the problem it tries to solve, and how it is solved."

The code changes themselves are good - they improve exception logging (logger.exception() instead of logger.error()), fix Pydantic field defaults (default_factory=list), and clean up formatting.

@Kludex Kludex merged commit 6f0e8e7 into main Jan 27, 2026
36 checks passed
@Kludex Kludex deleted the more-improvements branch January 27, 2026 12:48
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.

3 participants