Skip to content

Don't discourage implementing core::fmt::Write - #76750

Merged
bors merged 1 commit into
rust-lang:masterfrom
camelid:dont-discourage-core-fmt-write
Oct 9, 2020
Merged

Don't discourage implementing core::fmt::Write#76750
bors merged 1 commit into
rust-lang:masterfrom
camelid:dont-discourage-core-fmt-write

Conversation

@camelid

@camelidcamelid commented Sep 15, 2020•
edited by Mark-Simulacrum
Loading

Copy link
Copy Markdown
Member

Fixes#76729.

Explain when you should use it and when you should not.

@rustbotrustbot added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api [DEPRECATED; DO NOT USE] labels Sep 15, 2020
@rust-highfive

rust-highfive commented Sep 15, 2020

Copy link
Copy Markdown
Contributor

r? @Mark-Simulacrum

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2020
camelid
camelid commented Sep 15, 2020
View reviewed changes
Comment threadlibrary/core/src/fmt/mod.rs Outdated
@Mark-SimulacrumMark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2020
@camelid
camelidforce-pushed the dont-discourage-core-fmt-write branch from 76ed801 to ce01363CompareSeptember 27, 2020 23:37
@camelid

camelid commented Sep 27, 2020

Copy link
Copy Markdown
MemberAuthor

Rebased.

camelid
camelid commented Sep 27, 2020
View reviewed changes

@camelidcamelid left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@workingjubilee Is this a better description?

@camelidcamelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 27, 2020
Mark-Simulacrum
Mark-Simulacrum reviewed Sep 30, 2020
View reviewed changes
Comment threadlibrary/core/src/fmt/mod.rs Outdated
Comment threadlibrary/core/src/fmt/mod.rs Outdated
@Mark-SimulacrumMark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2020
@camelidcamelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 30, 2020
@workingjubilee

workingjubilee commented Sep 30, 2020

Copy link
Copy Markdown
Member

This looks good!

@camelid

camelid commented Sep 30, 2020

Copy link
Copy Markdown
MemberAuthor

Okay, ready for a final review!

@camelidcamelid added the A-fmt Area: `core::fmt` label Sep 30, 2020
@camelid

camelid commented Oct 2, 2020

Copy link
Copy Markdown
MemberAuthor

Squashed.

@camelid
camelidforce-pushed the dont-discourage-core-fmt-write branch from 9127f52 to 32ad7b2CompareOctober 2, 2020 18:51
@camelid

camelid commented Oct 2, 2020

Copy link
Copy Markdown
MemberAuthor

@Mark-Simulacrum could you re-review this? I addressed @workingjubilee's comments.

Mark-Simulacrum
Mark-Simulacrum reviewed Oct 3, 2020
View reviewed changes
Comment threadlibrary/core/src/fmt/mod.rs Outdated
@Mark-SimulacrumMark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2020
@camelidcamelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 4, 2020
@Mark-SimulacrumMark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 4, 2020
@Mark-SimulacrumMark-Simulacrum removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2020
@camelid

camelid commented Oct 4, 2020

Copy link
Copy Markdown
MemberAuthor

Hmm, I'm not sure why you added S-waiting-on-author; I'm waiting on a review for my newest commit.

@camelidcamelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 4, 2020
Mark-Simulacrum
Mark-Simulacrum reviewed Oct 4, 2020
View reviewed changes
Comment threadlibrary/core/src/fmt/mod.rs Outdated
@camelidcamelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 4, 2020
@camelid

camelid commented Oct 5, 2020

Copy link
Copy Markdown
MemberAuthor

@Mark-Simulacrum I shortened and simplified it, and I went with something similar to what you suggested.

@bors

This comment has been minimized.

Sign in to view
@camelid
camelidforce-pushed the dont-discourage-core-fmt-write branch from c40e1ab to 9e88286CompareOctober 5, 2020 21:20
@camelid

camelid commented Oct 5, 2020

Copy link
Copy Markdown
MemberAuthor

Rebased.

@Mark-Simulacrum

Mark-Simulacrum commented Oct 8, 2020

Copy link
Copy Markdown
Member

Please squash the commits, r=me with that done.

@camelid
Don't discourage implementing core::fmt::Write
c17d067
Explain when you should use it and when you should not.
@camelid

camelid commented Oct 8, 2020

Copy link
Copy Markdown
MemberAuthor

Squashed.

@camelid
camelidforce-pushed the dont-discourage-core-fmt-write branch from 9e88286 to c17d067CompareOctober 8, 2020 17:50
@Mark-Simulacrum

Mark-Simulacrum commented Oct 8, 2020

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Oct 8, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit c17d067 has been approved by Mark-Simulacrum

@borsbors 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 Oct 8, 2020
@jonas-schievinkjonas-schievink mentioned this pull request Oct 8, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 8, 2020
@bors
Auto merge ofrust-lang#77723- jonas-schievink:rollup-htz44r4, r=jon…
8a84c4f
…as-schievink
Rollup of 8 pull requests
Successful merges:
- rust-lang#76750 (Don't discourage implementing `core::fmt::Write`)
- rust-lang#77449 (BTreeMap: comment why drain_filter's size_hint is somewhat pessimistic)
- rust-lang#77660 ((docs): make mutex error comment consistent with codebase)
- rust-lang#77663 (Add compile fail test for issue 27675)
- rust-lang#77673 (Remove unnecessary lamda on emitter map.)
- rust-lang#77701 (Make `max_log_info` easily greppable (for figuring out why debug logging is disabled))
- rust-lang#77702 (Remove not needed lambda.)
- rust-lang#77710 (Update submodule llvm to get LVI bugfix)
Failed merges:
r? `@ghost`
@bors
bors merged commit 2766b72 into rust-lang:masterOct 9, 2020
@rustbotrustbot added this to the 1.49.0 milestone Oct 9, 2020
@camelid
camelid deleted the dont-discourage-core-fmt-write branch October 9, 2020 01:04
@camelidcamelid mentioned this pull request Oct 9, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mark-SimulacrumMark-SimulacrumMark-Simulacrum left review comments
@workingjubileeworkingjubileeworkingjubilee left review comments

Assignees

@Mark-SimulacrumMark-Simulacrum

Labels

A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-fmtArea: `core::fmt`S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-libs-api[DEPRECATED; DO NOT USE]

Projects

None yet

Milestone

1.49.0

Development

Successfully merging this pull request may close these issues.

Don't discourage people from using std::fmt::Write

6 participants

@camelid@rust-highfive@workingjubilee@bors@Mark-Simulacrum@rustbot