-
Notifications
You must be signed in to change notification settings - Fork 1k
Update for 1.5.5 release with Async Service Calls #3116
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3116 +/- ##
==========================================
+ Coverage 57.48% 57.74% +0.26%
==========================================
Files 357 357
Lines 69113 70350 +1237
Branches 14201 14540 +339
==========================================
+ Hits 39729 40625 +896
- Misses 25136 25416 +280
- Partials 4248 4309 +61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrsuciu
left a comment
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.
The newly added CallAsync overrides apparently intend to prioritize the OnCallAsync callbacks against the OnCall callbacks. If both callbacks are set than the apparent intent is not going to work and the OnCallAsync callback will be called for no reason before the OnCall callback. Shouldn't the OnCallAsync callback populate the output arguments and return the result imediatly ? Is my assumption about the priority intend wrong ?
mrsuciu
left a comment
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.
See comment regarding the possibility of OnCallAsync to be called unintendedly before OnCall if both are set.
Updated generated files.