-
Notifications
You must be signed in to change notification settings - Fork 83
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
GEN-37: Combine IntoReport
and ResultExt
#2883
GEN-37: Combine IntoReport
and ResultExt
#2883
Conversation
IntoReport
and ResultExt
IntoReport
and ResultExt
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2883 +/- ##
==========================================
- Coverage 51.63% 51.55% -0.09%
==========================================
Files 337 337
Lines 28971 29022 +51
Branches 433 433
==========================================
+ Hits 14960 14963 +3
- Misses 14009 14057 +48
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
IntoReport
and ResultExt
IntoReport
and ResultExt
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.
Minor things, otherwise looking good. Thank you!
Co-authored-by: Bilal Mahmoud <bmahmoud@mpi-cbg.de>
Also: We still need a changelog entry! |
Co-authored-by: Bilal Mahmoud <bmahmoud@mpi-cbg.de>
🌟 What is the purpose of this PR?
After a lot of back and forth and various discussions (internally and on GitHub and other public platforms), we decided to merge the functionality of
IntoReport
intoResultExt
.🔗 Related links
change_context
to callinto_report
#1968🔍 What does this change?
ResultExt
by atype Context: Context
associated type (technically a breaking change likely has no impact) - requires a minor version bumpResultExt
forcore::result::Result<T, C> where C: Context
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
IntoReportCompat
but due to the Orphan rule it's not possible to implementResultExt
oneyre
oranyhow
errors.