-
Notifications
You must be signed in to change notification settings - Fork 46
Make FutureResult non-Send when no-std
#335
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
base: master
Are you sure you want to change the base?
Make FutureResult non-Send when no-std
#335
Conversation
Pull Request Test Coverage Report for Build 18875521602Details
💛 - Coveralls |
ValuedMammal
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.
Concept ACK
|
@Fedeparma74 you should rebase your branch on top of master instead of merging. There's no need to have a merge commit on the master branch. |
63dfcf9 to
acc0c5f
Compare
Wooops, sorry! I forgot about this PR and quickly updated my fork as I needed it for another project. Rebased now :) |
|
Concept ACK. Verified that CI passes on my fork and looks ok. @ValuedMammal can you trigger CI? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #335 +/- ##
=======================================
Coverage 85.24% 85.24%
=======================================
Files 23 23
Lines 8229 8229
=======================================
Hits 7015 7015
Misses 1214 1214
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
ACK acc0c5f I'm not aware of anyone downstream who would be negatively affected. As you said, the change is non-breaking and backwards compatible. |
luisschwab
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.
ACK acc0c5f
Description
This PR updates the
FutureResulttype alias in the async persister implementation to support non-Sendfutures when thestdfeature is disabled.Previously, the
FutureResulttype always required theSendbound, which preventedno_stdenvironments from using non-Sendfutures. With this change, the bound is applied only when thestdfeature is enabled.This improves compatibility for embedded or
no_stdtargets while keeping fullSendsafety for standard environments.Notes to the reviewers
The change is minimal and fully backwards compatible.
It only affects compilation under
no_stdbuilds and does not modify any runtime behavior or APIs under thestdfeature.Please confirm that this approach aligns with the intended async API design and does not conflict with downstream usage expectations.
Changelog notice
Changed
FutureResulttype alias to allow non-Sendfutures inno_stdbuilds.