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

Add serde impls to LogLevel and LogLevelFilter #161

Merged
merged 2 commits into from May 30, 2017

Conversation

Projects
None yet
3 participants
@aergonaut
Copy link
Contributor

aergonaut commented May 21, 2017

Fixes #143

Adds serde as an optional dependency and puts the Serialize and Deserialize traits behind cfg_attr. I wasn't sure what version I should specify. 1.0.7 was the latest when I checked, but maybe it's too specific? Would >= 1.0 be better?

Saw the post on the internals forum, wanted to help out!

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented May 21, 2017

Thanks! Could you also add some CI configuration to test this on Travis/AppVeyor? Also, I think the dependency here is safe to just be serde = "1.0" as >= is actually a little too eager as it may or may not be compatible with 2.0 (we're not sure if it will be).

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 24, 2017

@aergonaut are you still working on this or would you need to hand it off to someone else?

@aergonaut

This comment has been minimized.

Copy link
Contributor Author

aergonaut commented May 24, 2017

I would still like to work on this! I will try to come back to it by the end of the week.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 24, 2017

No problem, let us know if you get stuck.

aergonaut added some commits May 21, 2017

@aergonaut aergonaut force-pushed the aergonaut:serde-impls branch from bbf4309 to 000a21b May 30, 2017

@aergonaut

This comment has been minimized.

Copy link
Contributor Author

aergonaut commented May 30, 2017

@dtolnay @alexcrichton I think this is ready now! I added cargo test --features serde to the Travis and Appveyor configs. I also fixed up the feature flag usage so it actually works.

@dtolnay dtolnay merged commit 000a21b into rust-lang-nursery:master May 30, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dtolnay
Copy link
Member

dtolnay left a comment

Thanks, I combined this with #181.

@aergonaut aergonaut deleted the aergonaut:serde-impls branch May 30, 2017

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.