-
Notifications
You must be signed in to change notification settings - Fork 10
tgb: CallbackDataFilter and CallbackDataCodec #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 94.91% 95.31% +0.40%
==========================================
Files 23 24 +1
Lines 2417 2626 +209
==========================================
+ Hits 2294 2503 +209
Misses 84 84
Partials 39 39 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mr-linch - I've reviewed your changes and they look great!
General suggestions:
- Consider adding more comprehensive documentation at the package level to outline the intended use cases, limitations, and examples of how to use the parser effectively.
- Enhance error messages to include field names where applicable, making it easier for developers to identify and fix issues.
- Review the default value for float precision and document the rationale behind the choice or consider a more intuitive default.
- Ensure that the error messages are descriptive enough to help developers quickly understand the nature of the issues encountered.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
No description provided.