-
Notifications
You must be signed in to change notification settings - Fork 2
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
⬆️ 🤖 - You are standing on my toes #497
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new DegiroHandler class for handling Degiro API interactions within the cefi/handler module. The changes include adding a new file degiro.py with the DegiroHandler class implementation and updating the init.py file to import this new handler (currently commented out). File-Level Changes
Tips
|
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 @mraniki - I've reviewed your changes - here's some feedback:
Overall Comments:
- The entire implementation is commented out, making it difficult to review. Please uncomment the code when it's ready for review and ensure all TODOs are addressed.
- Consider adding documentation and tests for the new DegiroHandler to ensure its functionality and ease of use.
- There seems to be a mix of async and sync operations in the code. Please ensure consistency in the asynchronous approach throughout the handler.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #497 +/- ##
==========================================
- Coverage 74.48% 69.26% -5.23%
==========================================
Files 7 8 +1
Lines 439 488 +49
==========================================
+ Hits 327 338 +11
- Misses 112 150 +38 ☔ View full report in Codecov by Sentry. |
⬆️ 🛠️(deps): update dependency pytest-asyncio to ^0.24.0
Auto Release
Summary by Sourcery
Add a new DegiroHandler class to support trading operations using the Degiro API, enhancing the system's capability to interact with Degiro for financial trading.
New Features: