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

Set is_human_readable to false #215

Closed
dtolnay opened this issue Oct 20, 2017 · 13 comments
Closed

Set is_human_readable to false #215

dtolnay opened this issue Oct 20, 2017 · 13 comments
Milestone

Comments

@dtolnay
Copy link
Contributor

dtolnay commented Oct 20, 2017

I have not released this yet but I will in the next few days. serde-rs/serde#1044

trait Serializer {
    /* ... */

    fn is_human_readable(&self) -> bool { true }
}

trait Deserializer<'de> {
    /* ... */

    fn is_human_readable(&self) -> bool { true }
}

Bincode should override these to false to get more compact representation for types like IpAddr and Datetime.

@TyOverby
Copy link
Collaborator

👍

@dtolnay
Copy link
Contributor Author

dtolnay commented Oct 22, 2017

@Marwes
Copy link
Contributor

Marwes commented Oct 23, 2017

Would there be any reason to let this be configureable? (ala #214 ?). I suppose the only reason one would not want the human readable representation is to preserve backwards compatibility with the format of older versions.

@dtolnay
Copy link
Contributor Author

dtolnay commented Oct 23, 2017

I would not attempt to make one version of bincode parse every bincode data ever written. People can depend on both 0.9 and 1.0, deserialize with 0.9, serialize with 1.0 if necessary to migrate data.

@dtolnay dtolnay added this to the 1.0 milestone Nov 1, 2017
@dtolnay
Copy link
Contributor Author

dtolnay commented Nov 1, 2017

Marking for 1.0 because doing this later would be a breaking change.

Marwes pushed a commit to Marwes/bincode that referenced this issue Nov 2, 2017
... by utilizing that bincode is not human readable.

Uses the changes in serde-rs/serde#1044 which
allows data formats to report that they are not human readable. This
lets certain types serialize themselves into a more compact form as they
know that the serialized form does not need to be readable.

Closes bincode-org#215

BREAKING CHANGE

This changes how types serialize themselves if they detect the
`is_human_readable` state.
Marwes pushed a commit to Marwes/bincode that referenced this issue Nov 2, 2017
... by utilizing that bincode is not human readable.

Uses the changes in serde-rs/serde#1044 which
allows data formats to report that they are not human readable. This
lets certain types serialize themselves into a more compact form as they
know that the serialized form does not need to be readable.

Closes bincode-org#215

BREAKING CHANGE

This changes how types serialize themselves if they detect the
`is_human_readable` state.
@TyOverby
Copy link
Collaborator

TyOverby commented Nov 2, 2017

@Marwes: there's no reason that this couldn't be configurable, but I'm wondering if we should just add it and make it the default before bincode hits 1.0.0.

TyOverby pushed a commit that referenced this issue Nov 20, 2017
... by utilizing that bincode is not human readable.

Uses the changes in serde-rs/serde#1044 which
allows data formats to report that they are not human readable. This
lets certain types serialize themselves into a more compact form as they
know that the serialized form does not need to be readable.

Closes #215

BREAKING CHANGE

This changes how types serialize themselves if they detect the
`is_human_readable` state.
@RReverser
Copy link

So what's the further plan for this? Is this going to wait for 1.0 or is there a chance this fix could be released earlier in minor update?

@TyOverby
Copy link
Collaborator

waiting for 1.0, which ideally will be released early next week.

@RReverser
Copy link

@TyOverby Obviously this is taking more than a week, which is understandable and I'm fine with, but could you please publish intermediate version with this fix?

@TyOverby
Copy link
Collaborator

@RReverser: published!

@RReverser
Copy link

@TyOverby Thank you!

@RReverser
Copy link

Oh, you published actual 1.0.0 release? Awesome!

@TyOverby
Copy link
Collaborator

Just for you @RReverser! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants