Skip to content

Conversation

@philipbl
Copy link
Member

No longer print log messages to stderr

No longer print log messages to stderr
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR centralizes logger configuration from runners/kiosk.py to main.py and switches from dual logging (stderr + file) to file-only logging.

  • Moved logger configuration from kiosk-specific runner to the main CLI entry point
  • Changed from stderr output to exclusively file-based logging at logs/sss.log
  • Applied consistent logging configuration across all CLI commands (simulator, kiosk, demo, test)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
main.py Added centralized logger configuration with file-only output, removed stderr logging
runners/kiosk.py Removed redundant logger configuration and startup banner that is now handled centrally

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +30 to +38
level=logger_level[verbose],
)

logger.info(" ____")
logger.info(" / . .\\")
logger.info(" \\ ---< Starting SSS")
logger.info(" \\ /")
logger.info(" __________/ /")
logger.info("-=:___________/")
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

With the default verbosity level (verbose=0), the logger level is set to "ERROR", which means these INFO level messages (including the ASCII art banner) won't be logged. Users would need to pass -vvv to see INFO messages or -vvvv for DEBUG messages. Consider setting a more appropriate default logging level (e.g., INFO) or documenting this behavior, as many important operational messages throughout the codebase use logger.info() and logger.debug().

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Copy link

Copilot AI commented Nov 14, 2025

@philipbl I've opened a new pull request, #90, to work on those changes. Once the pull request is ready, I'll request review from you.

@philipbl philipbl merged commit 5e1503d into dev Nov 14, 2025
11 of 15 checks passed
@philipbl philipbl deleted the fix_logging branch November 14, 2025 21:04
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