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

Mutable access to the top element of a BinaryHeap #33830

Conversation

moosingin3space
Copy link
Contributor

An implementation of the library change discussed here: rust-lang/rfcs#1626

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gankro (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.

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

cc @rust-lang/libs

/// *val = 0;
/// }
/// assert_eq!(heap.peek(), Some(&2));
///
Copy link
Member

Choose a reason for hiding this comment

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

Nit: extra line

@moosingin3space moosingin3space force-pushed the feature/mutable-high-priority-binaryheap branch 2 times, most recently from ae75365 to 6dcf641 Compare May 24, 2016 17:11
@alexcrichton
Copy link
Member

The libs team was able to discuss this PR today and the conclusion was that we'd like to merge, thanks for the PR @moosingin3space!

Could you be sure to update the tracking issue for the unstable APIs as well? If you open an issue I can be sure to tag it appropriately. Once that's done you can just squash the commits and I'll r+

@moosingin3space moosingin3space force-pushed the feature/mutable-high-priority-binaryheap branch from 6dcf641 to a51ad64 Compare June 21, 2016 05:22
@moosingin3space
Copy link
Contributor Author

@alexcrichton just opened a tracking issue and rebased.

/// }
/// assert_eq!(heap.peek(), Some(&2));
/// ```
#[unstable(feature = "binary_heap_peek_mut", issue = "0")]
Copy link
Member

Choose a reason for hiding this comment

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

Could you update these references to issue "0" to the tracking issue instead?

@moosingin3space moosingin3space force-pushed the feature/mutable-high-priority-binaryheap branch from a51ad64 to 2a34a7b Compare June 21, 2016 19:10
@moosingin3space
Copy link
Contributor Author

@alexcrichton all green and style issues fixed.

@alexcrichton
Copy link
Member

@bors: r+ 2a34a7b

Thanks!

@bors
Copy link
Collaborator

bors commented Jun 22, 2016

⌛ Testing commit 2a34a7b with merge ce2cd27...

@alexcrichton
Copy link
Member

@bors: retry force clean

  • restarted buildbot

@bors
Copy link
Collaborator

bors commented Jun 22, 2016

⌛ Testing commit 2a34a7b with merge 66b82be...

bors added a commit that referenced this pull request Jun 22, 2016
…binaryheap, r=alexcrichton

Mutable access to the top element of a BinaryHeap

An implementation of the library change discussed here: rust-lang/rfcs#1626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants