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 1-argument ensure!($expr) #86

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

sharnoff
Copy link
Contributor

Amends a subltle difference from anyhow. This PR is basically duplicated from dtolnay/anyhow#129.

@thenorili
Copy link
Contributor

thenorili commented Nov 5, 2023

Hi! This seems like a small and useful change, maintaining compatibility with anyhow's ensure! sounds worthwhile to me.

It looks like this failed CI due to some problems that weren't really your fault and now it's gone stale. Would you consider rebasing and updating the review now that tests are more stable and there are more folks around to handle contributions?

@sharnoff
Copy link
Contributor Author

sharnoff commented Nov 6, 2023

No worries :)

Rebased - no conflicts, didn't check if the changes are still valid

Copy link
Contributor

@thenorili thenorili left a comment

Choose a reason for hiding this comment

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

This looks good to me! I've checked it against the upstream version and 'ensure'd it looks okay =] I did not have formal review powers before but now I do, so here goes nothing: ✅

@thenorili thenorili merged commit ec98ce3 into eyre-rs:master Nov 21, 2023
29 checks passed
@thenorili
Copy link
Contributor

Thanks so much for the contribution! :D

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.

2 participants