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

Derive Debug for public structs #59

Merged
merged 3 commits into from Jul 31, 2017

Conversation

Projects
None yet
4 participants
@tmccombs
Copy link
Contributor

tmccombs commented Jun 25, 2017

Fixes #34

@KodrAus
Copy link
Contributor

KodrAus left a comment

Thanks @tmccombs! I've just left a simple comment about the WalkDirOptions Debug implementation.

src/lib.rs Outdated
@@ -195,6 +196,20 @@ struct WalkDirOptions {
contents_first: bool,
}

impl fmt::Debug for WalkDirOptions {
fn fmt(&self, f: &mut fmt::Formatter) -> ::std::result::Result<(), fmt::Error> {
write!(f, "WalkDirOptions {{ follow_links: {:?}, max_open: {:?}, \

This comment has been minimized.

@KodrAus

KodrAus Jun 26, 2017

Contributor

Do we need to manually implement Debug for the sort_by field?

There's a handy debug_struct method on Formatter that we could use here to simplify this implementation.

This comment has been minimized.

@BurntSushi

BurntSushi Jun 26, 2017

Owner

Yes, this should definitely be using debug_struct.

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Jun 26, 2017

Hmm, it looks like we might have to do some more Debug implementations manually to keep the code compiling on Rust 1.10.

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Jun 26, 2017

Looks like we'll bump the minimum version to 1.16 in #52, so that might fix the build failure.

@tmccombs tmccombs force-pushed the tmccombs:derive-debug branch from 2b2bff2 to 1370694 Jun 28, 2017

@tmccombs tmccombs force-pushed the tmccombs:derive-debug branch from 1370694 to f6ec352 Jul 3, 2017

@brson

This comment has been minimized.

Copy link

brson commented Jul 6, 2017

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Jul 31, 2017

Do you think this one is good to go now @BurntSushi?

@BurntSushi BurntSushi merged commit 079d145 into BurntSushi:master Jul 31, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Jul 31, 2017

@KodrAus Ah yup, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.