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
Add a shorthand for AppendInvoke #65
Conversation
Codecov Report
@@ Coverage Diff @@
## master #65 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 108 109 +1
=========================================
+ Hits 108 109 +1
|
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.
Thank you for the contribution! This seems like a valid API to add to multierr. I left a couple of suggestions re: the doc. Can you please also add some test cases that covers this?
23d7f37
to
a2bdee3
Compare
Applied suggestion docs changes. Added a test for the function. Let me know if you see other things to improve. |
Co-authored-by: Sung Yoon Whang <sungyoonwhang@gmail.com> Apply code review suggestions Co-authored-by: Sung Yoon Whang <sungyoonwhang@gmail.com> Adding tests
a2bdee3
to
e06fb97
Compare
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.
Thanks for making the changes. LGTM.
@abhinav do you know why the build is stuck here? |
I really like the idea of catching returned errors from deferred functions. Though having to use
multierr
package name twice in the same line makes it a bit verbose in many occasions.This PR introduces a shorthand for AppendInvoke which allows passing function or method value directly without wrapping it into an Invoker.
So this:
could become this: