Skip to content

fix: use logger instead of print() in events.py - #3965

Open
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/events-use-logger
Open

fix: use logger instead of print() in events.py#3965
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/events-use-logger

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

Three error/warning messages in events.py used print(..., file=sys.stderr) instead of the module's existing logger, bypassing logging configuration.

Fix

Replace print() calls with logger.error()/logger.warning() for consistent logging behavior.

…ts.py

Replace print(..., file=sys.stderr) calls with logger.error()/warning()
for consistent logging behavior. The module already defines
logger = logging.getLogger(__name__) and uses it elsewhere, but three
messages were using print() which bypasses logging configuration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Replaces direct stderr printing with logging in event handling.

Changes:

  • Logs dispatcher failures as errors.
  • Logs unsupported integration events as warnings.
Show a summary per file
File Description
src/specify_cli/events.py Updates event diagnostics to use logging.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/specify_cli/events.py
Comment on lines +279 to +282
logger.error("Event %s timed out", command_name)
return 2
except Exception as e:
print(f"Event {command_name} error: {e}", file=sys.stderr)
logger.error("Event %s error: %s", command_name, e)
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