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.

Sign up for GitHub

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

Jump to bottom

Add Vec::dedup_by and Vec::dedup_by_key #36743

Merged
merged 4 commits into from
Oct 14, 2016
Merged

Add Vec::dedup_by and Vec::dedup_by_key #36743

merged 4 commits into from
Oct 14, 2016

Conversation

SimonSapin
Copy link
Contributor

No description provided.

@SimonSapin
Remove duplicate test.
dc97341 
test_dedup_shared has been exactly the same as test_dedup_unique since
6f16df4, three years ago.
@SimonSapin
Move Vec::dedup tests from slice.rs to vec.rs
f14f4db
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@SimonSapin
Copy link
Contributor Author

In the spirit of #35856.

(Separate commits show smaller diffs.)

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 27, 2016
@alexcrichton
Copy link
Member

Seems like a reasonable addition to me!

cc @rust-lang/libs

@aturon
Copy link
Member

aturon commented Sep 27, 2016

I'm 👍 on this addition.

@alexcrichton
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Oct 3, 2016 •
edited
Loading

FCP proposed with disposition to merge. Review requested from:

No concerns currently listed.
See this document for info about what commands tagged team members can give me.

@alexcrichton
Copy link
Member

Also we'll need to be sure to add a tracking issue for these, but I'm fine with that happening just before merge

@SimonSapin
Copy link
Contributor Author

I’ll open a tracking issue and edit the PR to reference it when (if) this addition is accepted.

@alexcrichton
Copy link
Member

@SimonSapin ok, want to update with an unstable issue reference? I'll r+ after

@SimonSapin SimonSapin mentioned this pull request Oct 11, 2016
Closed
@SimonSapin
Add Vec::dedup_by and Vec::dedup_by_key
be34bac
@SimonSapin
Merge two impl<T> Vec<T> blocks.
401f1c4 
The show up separately in rustdoc.

This is a separate commit to keep the previous one’s diff shorter.
@SimonSapin
Copy link
Contributor Author

Done: #37087.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 11, 2016

📌 Commit 401f1c4 has been approved by alexcrichton

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Oct 11, 2016
Rollup merge of rust-lang#36743 - SimonSapin:dedup-by, r=alexcrichton
10d7e12 
Add Vec::dedup_by and Vec::dedup_by_key
@sophiajt sophiajt mentioned this pull request Oct 11, 2016
Closed
bors added a commit that referenced this pull request Oct 12, 2016
@bors
Auto merge of #37093 - jonathandturner:rollup, r=jonathandturner
ccb8b3e 
Rollup of 10 pull requests

- Successful merges: #36692, #36743, #36762, #36991, #37023, #37050, #37056, #37064, #37066, #37067
- Failed merges:
@bors
Copy link
Contributor

bors commented Oct 12, 2016

⌛ Testing commit 401f1c4 with merge 4655164...

@bors
Copy link
Contributor

bors commented Oct 12, 2016

💔 Test failed - auto-win-msvc-64-cargotest

@SimonSapin
Copy link
Contributor Author 

failures:

---- wrong_checksum_is_an_error stdout ----
    thread 'wrong_checksum_is_an_error' panicked at 'failed to remove dir C:\bot\slave\auto-win-msvc-64-cargotest\build\obj\build\ct\cargo\target\cit\t0\bar\target\debug\deps: The directory is not empty. (os error 145)', tests\cargotest\support\paths.rs:143
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    wrong_checksum_is_an_error

test result: FAILED. 6 passed; 1 failed; 0 ignored; 0 measured



command did not execute successfully: "C:\\bot\\slave\\auto-win-msvc-64-cargotest\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\cargotest.exe" "C:\\bot\\slave\\auto-win-msvc-64-cargotest\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage0/bin\\cargo.exe" "C:\\bot\\slave\\auto-win-msvc-64-cargotest\\build\\obj\\build\\ct"
expected success, got: exit code: 101


Makefile:51: recipe for target 'check-cargotest' failed

Is this an intermittent failure?

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Oct 13, 2016

⌛ Testing commit 401f1c4 with merge a37d74b...

bors added a commit that referenced this pull request Oct 13, 2016
@bors
Auto merge of #36743 - SimonSapin:dedup-by, r=alexcrichton
a37d74b 
Add Vec::dedup_by and Vec::dedup_by_key
@bors
Copy link
Contributor

bors commented Oct 13, 2016

💔 Test failed - auto-linux-cross-opt

@arielb1
Copy link
Contributor

arielb1 commented Oct 13, 2016

@bors retry

@bors
Copy link
Contributor

bors commented Oct 13, 2016

⌛ Testing commit 401f1c4 with merge 3ab1839...

@bors
Copy link
Contributor

bors commented Oct 13, 2016

💔 Test failed - auto-linux-cross-opt

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Oct 14, 2016

⌛ Testing commit 401f1c4 with merge 17af6b9...

bors added a commit that referenced this pull request Oct 14, 2016
@bors
Auto merge of #36743 - SimonSapin:dedup-by, r=alexcrichton
17af6b9 
Add Vec::dedup_by and Vec::dedup_by_key
@bors
Copy link
Contributor

bors commented Oct 14, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: alexcrichton
Pushing 17af6b9 to master...

@bors bors merged commit 401f1c4 into rust-lang:master Oct 14, 2016
@rfcbot
Copy link

rfcbot commented Oct 17, 2016

All relevant subteam members have reviewed. No concerns remain.

@rfcbot
Copy link

rfcbot commented Oct 24, 2016

It has been one week since all blocks to the FCP were resolved.

@SimonSapin SimonSapin deleted the dedup-by branch December 7, 2016 03:15
@mbrubeck mbrubeck mentioned this pull request Sep 29, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@SimonSapin @rust-highfive @alexcrichton @aturon @rfcbot @bors @arielb1