-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Stabilise vec::remove_item #67727
Stabilise vec::remove_item #67727
Conversation
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
0890318
to
63fb61b
Compare
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.
I'm pretty sure, that the new test doesn't work in the current form 😄
@jfrimmel ye you right :D had a gcc issue so just went with the tests in blind form waiting for the CI to tell me what's wrong :P fixed everything now. Thanks for the review |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
spurious... @bors retry |
@bors try |
⌛ Trying commit 73ba2540a4fbc617d4e2a40e69c2c17fc6f2e4bf with merge 4e459c65afc2d20e94fcfa4ba6b68bccccca098e... |
☀️ Try build successful - checks-azure |
Many people suggested in the tracking issue to have this method take a let mut vec = vec![String::from("foo")];
vec.remove_item("foo");
// ^^^^^ expected struct `std::string::String`, found `str` |
Created a new PR that made that change. Blocked on #67873 |
…partial, r=Amanieu change remove to have a PartialEq bound Addresses [comment](rust-lang#67727 (comment)). References rust-lang#40062 r? @Amanieu
📌 Commit 6bec8e9 has been approved by |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors: r- (CI failures) |
ye forgot to remove 1 feature gate.. pushing it now :) |
@bors: r+ |
📌 Commit 503d06b has been approved by |
…alexcrichton Stabilise vec::remove_item Closes rust-lang#40062 r? @alexcrichton
…alexcrichton Stabilise vec::remove_item Closes rust-lang#40062 r? @alexcrichton
Rollup of 13 pull requests Successful merges: - #67566 (Add an unstable conversion from thread ID to u64) - #67671 (Account for `type X = impl Trait;` in lifetime suggestion) - #67727 (Stabilise vec::remove_item) - #67877 (Omit underscore constants from rustdoc) - #67880 (Handle multiple error fix suggestions carefuly) - #67898 (Improve hygiene of `newtype_index`) - #67908 (rustdoc: HTML escape const values) - #67909 (Fix ICE in const pretty printing and resolve FIXME) - #67929 (Formatting an example for method Vec.retain) - #67934 (Clean up E0178 explanation) - #67936 (fire "non_camel_case_types" for associated types) - #67943 (Missing module std in example.) - #67962 (Update books) Failed merges: r? @ghost
Please don't stabilise it without fixing the name! Please don't call it just |
Unstabilize `Vec::remove_item` As concerned by @kornelski, @LukasKalbertodt, and @gnzlbg in rust-lang#40062. Reverts rust-lang#67727
Unstabilize `Vec::remove_item` As concerned by @kornelski, @LukasKalbertodt, and @gnzlbg in rust-lang#40062. Reverts rust-lang#67727
Closes #40062
r? @alexcrichton