Skip to content

Conversation

@smiyashaji
Copy link
Contributor

Summary

This PR replaces all print statements across the codebase with logging module calls for structured logging. This makes future debugging, maintenance, and integration with tooling significantly easier.

Changes

  • Added logger instances where required.
  • Used logging.basicConfig() for CLI entry point.
  • Replaced all stdout prints with appropriate logging levels.

Ready for review.

@devdanzin
Copy link
Owner

Thank you very much for this PR and sorry for taking so long to respond.

The wanted changes are great, thank you! But unfortunately we can't apply this PR as it currently stands because it makes too many unwanted changes: it removes module docstrings and many code comments, and even makes small unrelated fixes. I'd love to help you getting this PR merged, so let's discuss how to fix these issues.

We have some options for keeping only the wanted changes:

  • If you used AI to make these changes, you could try making the same request but specifying it shouldn't modify any docstrings or code comments.
  • You could manually remove the unwanted deletions instead, which is a lot more work but also makes you more acquainted with the codebase.
  • I could send a PR to your fork undoing the unwanted changes. While this would mean less work for you (and more for me 😊), I think this would rob you a great opportunity to learn how to fix a PR you submitted.
  • Any other ideas? I'm open to suggestions.

So, which solution would you prefer?

Thanks again for the PR!

@devdanzin
Copy link
Owner

Hey @smiyashaji, do you still want to work on this PR? No hurry, let me know if you need help with anything. There are some conflicts with the main branch now, I can help you get these sorted if you want.

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