Skip to content
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 IntoStackFutureError #21

Merged
merged 4 commits into from
Nov 16, 2022
Merged

Add IntoStackFutureError #21

merged 4 commits into from
Nov 16, 2022

Conversation

eholk
Copy link
Contributor

@eholk eholk commented Nov 9, 2022

This is perhaps an over-designed error type, but it provides enough information to figure out why the conversion failed, which can help the programmer fix it.

This PR also includes #20, so review that first and I'll merge that one and rebase this one before merging it.

It would be nice to add a impl std::error::Error for IntoStackFutureError, but for now we are a no_std crate and I wasn't sure adding a no_std feature would be worth it just for that impl. I think the plan is for the Error trait to become part of core someday, so we could just wait until then.

@eholk eholk requested a review from daprilik November 9, 2022 01:26
Copy link
Collaborator

@daprilik daprilik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broad strokes seem good, though I had a few questions to resolve before smashing that approve button

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
eholk and others added 3 commits November 16, 2022 15:08

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: Daniel Prilik <71350465+daprilik@users.noreply.github.com>
@eholk eholk merged commit 0856ccb into microsoft:main Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants