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 an impl for Box<Error> from String. #30509

Merged
merged 2 commits into from Jan 13, 2016

Conversation

Projects
None yet
8 participants
@michaelsproul
Copy link
Contributor

michaelsproul commented Dec 21, 2015

Closes #30156.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Dec 21, 2015

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 21, 2015

@@ -119,6 +119,15 @@ impl From<String> for Box<Error + Send + Sync> {
}
}

#[unstable(feature = "string_box_error", reason = "recently added", issue = "30156")]

This comment has been minimized.

@sfackler

sfackler Dec 22, 2015

Member

Implementations aren't checked for stability so this should really be flagged stable.

This comment has been minimized.

@michaelsproul

michaelsproul Dec 22, 2015

Author Contributor

I was wondering about that. I'll amend the commit and do a push.

@michaelsproul

This comment has been minimized.

Copy link
Contributor Author

michaelsproul commented Dec 28, 2015

@alexcrichton this seems to have stalled (no acknowledgement from @bors), and the r+ needs to be on the new commit. It's holidays though so take it easy! 😄

@netvl

This comment has been minimized.

Copy link
Contributor

netvl commented Jan 9, 2016

Is there a reason why there is no similar implementation for &str?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 11, 2016

@michaelsproul oh if the commit is updated it needs a new r+

As @netvl says actually, could you add an impl for &str as well?

@michaelsproul

This comment has been minimized.

Copy link
Contributor Author

michaelsproul commented Jan 12, 2016

Will fix it up soon.

@michaelsproul

This comment has been minimized.

Copy link
Contributor Author

michaelsproul commented Jan 13, 2016

Updated! I really like the &str implementation, great idea @netvl.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 13, 2016

@bors: r+ c1e527f

cc @rust-lang/libs, just a heads up, seems easy though as we have From<&str> for Box<Error+Send+Sync> so it seems like there's no reason to not also have it for Box<Error>

bors added a commit that referenced this pull request Jan 13, 2016

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 13, 2016

⌛️ Testing commit c1e527f with merge ac9be00...

@bors bors merged commit c1e527f into rust-lang:master Jan 13, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jan 16, 2016

bhsausabsha on IRC asked for a backport

@bluss

This comment has been minimized.

Copy link
Contributor

bluss commented Jan 19, 2016

backporting would solve this beta regression IIUC #30634

@brson brson added the beta-accepted label Jan 19, 2016

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jan 19, 2016

Per @nikomatsakis accepted for backport. This fixes another regression.

@brson brson removed the beta-nominated label Jan 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.