Skip to content

Conversation

Perdiga
Copy link
Collaborator

@Perdiga Perdiga commented Jul 9, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 00:30
Copilot

This comment was marked as outdated.

@Perdiga Perdiga requested a review from Copilot July 9, 2025 01:19
Copy link

@Copilot 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 refactors get_logger to make the logging level optional, allowing it to inherit from the root logger, and updates documentation and handler-level logic accordingly.

  • Changed level parameter to Optional[int] with a None default to inherit from the root logger
  • Added logic to set the logger level to NOTSET when level is None, and adjusted handler-level assignment
  • Updated docstring to explain the new behavior
Comments suppressed due to low confidence (2)

src/codeql_wrapper/infrastructure/logger.py:26

  • [nitpick] Consider clarifying in the docstring that configure_logging (or a call to basicConfig) must be invoked before get_logger so that the root logger’s level is set as expected when level is None.
        level: Logging level (if None, inherits from root logger)

src/codeql_wrapper/infrastructure/logger.py:39

  • Add unit tests for both branches of the level parameter (when it is None and when it is a concrete value) to verify that the logger and handler levels are correctly set or inherited.
    if level is not None:

@Perdiga Perdiga merged commit 75234b4 into main Jul 9, 2025
7 checks passed
@Perdiga Perdiga changed the title Refactor get_logger to improve level handling and documentation Fix level handling on logger Jul 9, 2025
@fernandosantos-br fernandosantos-br deleted the develop-improve-logger branch July 21, 2025 21:46
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