search

fitzgen / inlinable_string

An owned, grow-able UTF-8 string that stores small strings inline and avoids heap-allocation.
http://fitzgen.github.io/inlinable_string/inlinable_string/index.html
Other
69 stars 11 forks source link

Add serde to InlinableString #12

Closed KodrAus closed 7 years ago

KodrAus commented 7 years ago

Closes #11

This adds an optional serde feature for serialising/deserialising InlinableStrings. The implementation is really simple; it just uses Deref for serialisation and From for deserialisation.

KodrAus commented 7 years ago

@ArtemGr We should probably update CI to test this feature. I'm not sure what approach you all want for that though. Any thoughts?

ArtemGr commented 7 years ago

We should probably update CI to test this feature. I'm not sure what approach you all want for that though. Any thoughts?

I'd vouch for "cargo test --features=serde", because "cargo test --all-features" fails for me with "can't find crate" in "clippy-0.0.27\src\lib.rs", and that's a separate issue.

KodrAus commented 7 years ago

Ok, I've updated the travis config. What I've done:

I'm happy to tweak the build config however you want, I understand different projects like to do things in different ways :)

KodrAus commented 7 years ago

@fitzgen @ArtemGr Any more feedback on this one?

ArtemGr commented 7 years ago

Hey, @KodrAus, sorry for the delay: I was thinking that @fitzgen would pull the switch, but given the time that has passed I think he's simply okay with the patch.

Thank you, good work on the pull request!

ArtemGr commented 7 years ago

Do you need a version bump?

KodrAus commented 7 years ago

Awesome, thanks @ArtemGr!

This should be safe as a patch, right? Since the feature isn't available by default.

ArtemGr commented 7 years ago

You're welcome!

I've filed the version bump, https://github.com/fitzgen/inlinable_string/pull/13. We should wait for @fitzgen to approve.

P.S. And if you will make a version bump pull request of your own, then I think I will be able to merge it right away. That is, in case you need it sooner than later.

fitzgen commented 7 years ago

Hey! Sorry, somehow I missed this PR!