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

Tracking issue for {HashMap, BTreeMap}::get_key_value stabilization #49347

Closed
Diggsey opened this issue Mar 24, 2018 · 12 comments · Fixed by #64836
Closed

Tracking issue for {HashMap, BTreeMap}::get_key_value stabilization #49347

Diggsey opened this issue Mar 24, 2018 · 12 comments · Fixed by #64836
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Diggsey
Copy link
Contributor

Diggsey commented Mar 24, 2018

Implemented in #49346

@pietroalbini pietroalbini added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Mar 27, 2018
@bluss
Copy link
Member

bluss commented Apr 28, 2018

Can we add a mutable equivalent? Like fn get_key_value_mut<Q>(&mut self, &Q) -> Option<(&K, &mut V)>, or has that already been discussed?

@Diggsey
Copy link
Contributor Author

Diggsey commented Apr 28, 2018

@bluss it was discussed a little here: #46992 (comment)

It was just less clear cut what the API should look like.

@joshlf
Copy link
Contributor

joshlf commented Feb 6, 2019

Any update on this? I'd like to use this on stable.

@CAD97
Copy link
Contributor

CAD97 commented Jul 23, 2019

Is anything keeping this from stabilization?

@ErichDonGubler
Copy link
Contributor

ErichDonGubler commented Aug 2, 2019

Following up with what @Diggsey said, my summarized understanding is this: the get_key_value API for immutable access (which seems to be the entirety of what's being tracked here) seems to have borne no objections, but it was less clear if simply exposing a similar API that merely returns a tuple of mutable references and calling it get_key_value_mut would be preferred over reusing the OccupiedEntry API (i.e., returning Option<OccupiedEntry<_, _>>.

I don't see how the above influences this particular issue, so AFAICT there's nothing keeping this from stabilization at this point. @Diggsey, I don't know who else we could ping to get this pushed through...and I'd like to see this on stable too. :P

@Diggsey
Copy link
Contributor Author

Diggsey commented Aug 2, 2019

I believe this needs someone from the @rust-lang/libs team to propose an "fcp merge", or is there something else I need to do here?

On a meta-note, this does seem to be a bit of a gap in the process, with almost 500 open tracking issues, some of which have been sitting around since 2013 and no indication of who is responsible for next steps... Compare to eg. #60602 where the author is on the libs team and able to take a feature from implementation to stabilisation in a single release cycle.

If the stabilisation of this feature is simply not a priority, it would be nice to make that decision publicly.

@sfackler
Copy link
Member

sfackler commented Aug 2, 2019

The general process is that people who care about a feature ask for it to be stabilized. If no one cares enough to ask about it, it's probably not going to be stabilized.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Aug 2, 2019

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 2, 2019
@SimonSapin
Copy link
Contributor

SimonSapin commented Aug 2, 2019

@Diggsey I do sometimes look through https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AC-tracking-issue+label%3AT-libs and try to find tracking issues where there is nothing else left to do other than start FCP. If you do find some like that, feel free to ping someone in the team so they can do it. However in a lot of cases there are ongoing design discussions without a clear resolution. In some cases a tracking issue has many comments, and its not obvious whether or not all the issues that have been brought up are in fact resolved even though discussion has stalled. For those, it can be helpful to have someone read through the history of that feature (including potential other threads, like in an RFC or on internals.rlo), write up a summary comment, and propose a way forward.

@SimonSapin
Copy link
Contributor

Oh and then there’s https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AC-tracking-issue+label%3Afinished-final-comment-period+label%3Adisposition-merge where the only thing left to do might be a stabilization PR (which anyone can submit). There are some false positives there, though.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 5, 2019
@rfcbot
Copy link

rfcbot commented Aug 5, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 15, 2019
@rfcbot
Copy link

rfcbot commented Aug 15, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants