refactor: replace print() with logging in YouTubeConverter#1797
Open
ahmedezzat2018 wants to merge 4 commits intomicrosoft:mainfrom
Open
refactor: replace print() with logging in YouTubeConverter#1797ahmedezzat2018 wants to merge 4 commits intomicrosoft:mainfrom
ahmedezzat2018 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
VANDRANKI
reviewed
Apr 19, 2026
VANDRANKI
left a comment
There was a problem hiding this comment.
Clean refactor - using the module logger instead of print() is the right pattern for library code. Callers can configure the markitdown.converters._youtube_converter logger to control verbosity.
One nit: there is trailing whitespace on the logger = logging.getLogger(__name__) line. Not functional but will likely fail linting.
Author
|
@microsoft-github-policy-service agree |
Author
|
Thanks for the feedback! I've removed the trailing whitespaceand reformatted the code using black. Looking forward to your thoughts |
VANDRANKI
approved these changes
Apr 20, 2026
VANDRANKI
left a comment
There was a problem hiding this comment.
Trailing whitespace gone and black formatting applied consistently. All three print() calls correctly replaced with logger.warning(). LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: YouTubeConverter used print() for error messages instead of
the standard logging module, bypassing the app's logging configuration.
Solution: Added import logging and a module-level logger, then replaced
all print() calls with logger.warning().