Skip to content

refactor: replace print() with logging in YouTubeConverter#1797

Open
ahmedezzat2018 wants to merge 4 commits intomicrosoft:mainfrom
ahmedezzat2018:fix/invalid-ooxml-raises-exception
Open

refactor: replace print() with logging in YouTubeConverter#1797
ahmedezzat2018 wants to merge 4 commits intomicrosoft:mainfrom
ahmedezzat2018:fix/invalid-ooxml-raises-exception

Conversation

@ahmedezzat2018
Copy link
Copy Markdown

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().

Copy link
Copy Markdown

@VANDRANKI VANDRANKI left a comment

Choose a reason for hiding this comment

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

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.

@ahmedezzat2018
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@ahmedezzat2018
Copy link
Copy Markdown
Author

Thanks for the feedback! I've removed the trailing whitespaceand reformatted the code using black. Looking forward to your thoughts

Copy link
Copy Markdown

@VANDRANKI VANDRANKI left a comment

Choose a reason for hiding this comment

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

Trailing whitespace gone and black formatting applied consistently. All three print() calls correctly replaced with logger.warning(). LGTM.

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.

2 participants