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

Stabilize Once::is_completed #68945

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Stabilize Once::is_completed #68945

merged 1 commit into from
Feb 20, 2020

Conversation

mjbshaw
Copy link
Contributor

@mjbshaw mjbshaw commented Feb 8, 2020

Closes #54890

This function has been around for some time. I haven't seen anyone raise any objections to it. I've personally found it useful myself. It would be nice to finally stabilize it and

@rust-highfive
Copy link
Collaborator

r? @LukasKalbertodt

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2020
LukasKalbertodt
LukasKalbertodt approved these changes Feb 8, 2020
View reviewed changes
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

@rfcbot fcp merge (@rust-lang/libs can someone do this for me? I am not allowed to)

What is stabilized here? std::sync::Once::is_completed:

pub fn is_completed(&self) -> bool

Implemented 1.5 years ago. The tracking issue did not bring up any concerns, so I think this is a pretty straight forward addition and I don't see why it shouldn't be stabilized.

@Mark-Simulacrum
Copy link
Member

The PR originally implementing this also mentioned that doing anything meaningful when the method returns false is likely wrong; do we want to add that to the docs prior to stabilization?

@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 8, 2020
@jonas-schievink jonas-schievink added this to the 1.43 milestone Feb 8, 2020
@mjbshaw
Copy link
Contributor Author

mjbshaw commented Feb 8, 2020

@Mark-Simulacrum I incorporated @nagisa's wording in an attempt to make that clearer. I'm hoping the updated verbiage is sufficient. I'm reluctant to explicitly say "don't do anything meaningful if this returns false" because I think that can be easily misunderstood to mean that the else branch in if once.is_completed() { ... } else { ... } is bad, when in reality it can be perfectly reasonable (i.e., 1, 2).

@Mark-Simulacrum
Copy link
Member

Yes that seems better.

@Amanieu
Copy link
Member

Amanieu commented Feb 9, 2020

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Feb 9, 2020 •
edited by KodrAus
Loading

Team member @Amanieu 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 Feb 9, 2020
@LukasKalbertodt

This comment has been minimized.

Sign in to view
@mjbshaw

This comment has been minimized.

Sign in to view
@LukasKalbertodt

This comment has been minimized.

Sign in to view
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Feb 9, 2020
@rfcbot
Copy link

rfcbot commented Feb 9, 2020

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Feb 9, 2020
@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 Feb 19, 2020
@rfcbot
Copy link

rfcbot commented Feb 19, 2020

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.

@LukasKalbertodt
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 20, 2020

📌 Commit 348278a has been approved by LukasKalbertodt

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2020
@Dylan-DPC
Rollup merge of rust-lang#68945 - mjbshaw:once_is_completed, r=LukasK…
2a3ce7b 
…albertodt

Stabilize Once::is_completed

Closes rust-lang#54890

This function has been around for some time. I haven't seen anyone raise any objections to it. I've personally found it useful myself. It would be nice to finally stabilize it and
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 20, 2020
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 20, 2020
Merged
bors added a commit that referenced this pull request Feb 20, 2020
@bors
Auto merge of #69309 - Dylan-DPC:rollup-gjdqx7l, r=Dylan-DPC
93711d0 
Rollup of 5 pull requests

Successful merges:

 - #68705 (Add LinkedList::remove())
 - #68945 (Stabilize Once::is_completed)
 - #68978 (Make integer exponentiation methods unstably const)
 - #69266 (Fix race condition when allocating source files in SourceMap)
 - #69287 (Clean up E0317 explanation)

Failed merges:

r? @ghost
@bors bors merged commit 588f008 into rust-lang:master Feb 20, 2020
@jplatte
Copy link
Contributor

jplatte commented Mar 12, 2020

This PR has the GitHub milestone 1.43 but the stabilization attribute says 1.44.0. Which one is correct?

@LukasKalbertodt LukasKalbertodt mentioned this pull request Mar 15, 2020
Merged
Centril added a commit to Centril/rust that referenced this pull request Mar 15, 2020
@Centril
Rollup merge of rust-lang#70018 - LukasKalbertodt:fix-once-is-complet…
bde77af 
…e-since, r=Centril

Fix "since" field for `Once::is_complete`'s `#[stable]` attribute

It was accidentally merged with the wrong version in rust-lang#68945.  Thanks @jplatte for noticing.

This also needs to be beta backported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukasKalbertodt LukasKalbertodt LukasKalbertodt approved these changes

Assignees

@LukasKalbertodt LukasKalbertodt

Labels
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. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.43
Development

Successfully merging this pull request may close these issues.

Tracking issue for std::sync::Once::is_completed
9 participants
@mjbshaw @rust-highfive @Mark-Simulacrum @Amanieu @rfcbot @LukasKalbertodt @bors @jplatte @jonas-schievink