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

std::sync::mpsc: Add fmt::Debug stubs #30894

Merged
merged 1 commit into from
Jan 20, 2016
Merged

Conversation

antrik
Copy link
Contributor

@antrik antrik commented Jan 14, 2016

Minimal fix for #30563

This covers all the public structs I think; except for Iter and
IntoIter, which I don't know if or how they should be handled.

Minimal fix for rust-lang#30563

This covers all the public structs I think; except for Iter and
IntoIter, which I don't know if or how they should be handled.
@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 @brson (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.

@jwilm
Copy link

jwilm commented Jan 15, 2016

Yay! I just ran into this problem. Glad to see a fix 😄.

@brson
Copy link
Contributor

brson commented Jan 16, 2016

cc @rust-lang/libs This seems reasonable to me. Do these implementations follow existing conventions for Debug for opaque types?

@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 16, 2016
@sfackler
Copy link
Member

I don't think I've never seen the { ... } suffix before.

@alexcrichton
Copy link
Member

I'm fine with this, and at least in my opinion I don't think that the exact output of Debug really matters in terms of backwards compatibility guarantees at all, so I don't mind whatever format is used.

A quick grep shows up one other instance of this.

@bluss
Copy link
Member

bluss commented Jan 16, 2016

Another convention says that debug impls don't need to indicate type information. These seem like sensible implementations anyway, just empty braces would just bring confused issue reports.

@brson
Copy link
Contributor

brson commented Jan 20, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Jan 20, 2016

📌 Commit 0e3fb18 has been approved by brson

bors added a commit that referenced this pull request Jan 20, 2016
Minimal fix for #30563

This covers all the public structs I think; except for Iter and
IntoIter, which I don't know if or how they should be handled.
@bors
Copy link
Contributor

bors commented Jan 20, 2016

⌛ Testing commit 0e3fb18 with merge 7561466...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants