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

added .collect() into String from Box<str> #72688

Merged
merged 1 commit into from
Jul 6, 2020
Merged

added .collect() into String from Box<str> #72688

merged 1 commit into from
Jul 6, 2020

Conversation

djugei
Copy link
Contributor

@djugei djugei commented May 28, 2020 •
edited
Loading

I have not created an rfc, because i felt like this is a very minor change.

i have just set a random feature name and rust version as stability attribute, i expect to have to change that, i just don't know what the policy on that is. all guides i could find focused on contributing to the compiler, not contributing to the standard library.

drawbacks: more code in the standard library, could be replaced with specialization: base-implementation for AsRef<str> and specialization for String and Cow. i can write that code if ppl want it.

advantages: using "real strings" i.e. Box<str> is as ergonomic as string slices (&str) and string buffers (String) with iterators.

@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 @joshtriplett (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2020
@djugei djugei marked this pull request as draft May 28, 2020 11:00
@djugei djugei marked this pull request as ready for review May 28, 2020 11:00
@Elinvynia Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2020
@Dylan-DPC-zz
Copy link

r? @Amanieu

@Amanieu
Copy link
Member

Amanieu commented Jun 23, 2020

@rfcbot fcp merge

@Amanieu Amanieu added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 23, 2020
@Amanieu
Copy link
Member

Amanieu commented Jun 23, 2020

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jun 23, 2020 •
edited by sfackler
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. 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 Jun 23, 2020
@rfcbot
Copy link

rfcbot commented Jun 23, 2020

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

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Jul 3, 2020
@rfcbot
Copy link

rfcbot commented Jul 3, 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.

@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 3, 2020
@Amanieu
Copy link
Member

Amanieu commented Jul 4, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 4, 2020

📌 Commit b4337ab has been approved by Amanieu

@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 Jul 4, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 4, 2020
@Manishearth
Rollup merge of rust-lang#72688 - djugei:master, r=Amanieu
1920a57 
added .collect() into String from Box<str>

I have not created an rfc, because i felt like this is a very minor change.

i have just set a random feature name and rust version as stability attribute, i expect to have to change that, i just don't know what the policy on that is. all guides i could find focused on contributing to the compiler, not contributing to the standard library.

drawbacks: more code in the standard library, could be replaced with specialization: base-implementation for AsRef\<str> and specialization for String and Cow. i can write that code if ppl want it.

advantages: using "real strings" i.e. Box\<str> is as ergonomic as string slices (&str) and string buffers (String) with iterators.
@Manishearth Manishearth mentioned this pull request Jul 4, 2020
Closed
@bors
Copy link
Contributor

bors commented Jul 4, 2020

⌛ Testing commit b4337ab with merge 5b3d23acec123dae4fe10b4d5329937b67d463f4...

@bors
Copy link
Contributor

bors commented Jul 4, 2020

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 4, 2020
@JohnTitor
Copy link
Member

@bors retry

@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 Jul 5, 2020
@bors
Copy link
Contributor

bors commented Jul 5, 2020

⌛ Testing commit b4337ab with merge fe8ef269261812c69bbf72d17cbfe076a6c8b7fa...

@Manishearth Manishearth mentioned this pull request Jul 5, 2020
Merged
@Manishearth
Copy link
Member

@bors retry yield

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 5, 2020
@bors
Auto merge of rust-lang#74073 - Manishearth:rollup-faqo9lx, r=Manishe…
0c03aee 
…arth

Rollup of 12 pull requests

Successful merges:

 - rust-lang#72688 (added .collect() into String from Box<str>)
 - rust-lang#73787 (Add unstable docs for rustc_attrs)
 - rust-lang#73834 (Some refactoring around intrinsic type checking)
 - rust-lang#73871 (Fix try_print_visible_def_path for Rust 2018)
 - rust-lang#73937 (Explain exhaustive matching on {usize,isize} maximum values)
 - rust-lang#73973 (Use `Span`s to identify unreachable subpatterns in or-patterns)
 - rust-lang#74000 (add `lazy_normalization_consts` feature gate)
 - rust-lang#74025 (Remove unnecessary release from Arc::try_unwrap)
 - rust-lang#74027 (Convert more `DefId`s to `LocalDefId`s)
 - rust-lang#74055 (Fix spacing in Iterator fold doc)
 - rust-lang#74057 (expected_found `&T` -> `T`)
 - rust-lang#74064 (variant_count: avoid incorrect dummy implementation)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jul 6, 2020

⌛ Testing commit b4337ab with merge 0c03aee...

@bors bors merged commit 5311daa into rust-lang:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

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. 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
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@djugei @rust-highfive @Dylan-DPC-zz @Amanieu @rfcbot @bors @JohnTitor @Manishearth @joshtriplett @Elinvynia