gh-uuid-rs/uuid
Watch
1
0
Fork
You've already forked uuid
 0
Code Issues 10 Pull requests 2 Releases Activity

Serialises UUID as [u8:16] #331

Merged
Redrield merged 8 commits from feature/dense_serde into master 2018-10-03 11:22:42 -05:00
Conversation 21 Commits 8 Files changed 5 +130 -2
Redrield commented 2018-10-01 19:32:22 -05:00 (Migrated from github.com)

I'm submitting a feature

Description

Adds the ability to serialize a Uuid as a [u8; 16]. Functionality is behind the dense_serde feature. the serde feature continues to serialize as a &[u8]. Both features serialize strings identically

Tests

A new test module for the feature was added, mirroring the tests of the serde feature (with modifications to expected tokens where necessary)

Related Issue(s)

closes #329

**I'm submitting a** feature # Description Adds the ability to serialize a Uuid as a [u8; 16]. Functionality is behind the `dense_serde` feature. the `serde` feature continues to serialize as a &[u8]. Both features serialize strings identically # Tests A new test module for the feature was added, mirroring the tests of the `serde` feature (with modifications to expected tokens where necessary) # Related Issue(s) closes #329
radix (Migrated from github.com) reviewed 2018-10-01 19:32:22 -05:00
KodrAus (Migrated from github.com) reviewed 2018-10-01 19:32:22 -05:00
KodrAus commented 2018-10-02 09:20:06 -05:00 (Migrated from github.com)

Thanks @Redrield!

From the discussion on #329, using a feature flag here doesn't actually save us from potential breakage. It looks like we want to keep the output of serde the same for anyone that's currently depending on it.

One way we could do this in a backwards compatible way that users properly opt into would be to convert your new Serialize and Deserialize impls into free functions, that can be used with the #[serde(serialize_with)] and #[serde(deserialize_with)] attributes.

Thanks @Redrield! From the discussion on #329, using a feature flag here doesn't actually save us from potential breakage. It looks like we want to keep the output of `serde` the same for anyone that's currently depending on it. One way we could do this in a backwards compatible way that users properly opt into would be to convert your new `Serialize` and `Deserialize` impls into free functions, that can be used with the `#[serde(serialize_with)]` and `#[serde(deserialize_with)]` attributes.
kinggoesgaming commented 2018-10-02 10:14:11 -05:00 (Migrated from github.com)

@KodrAus newtypes?

@KodrAus newtypes?
smarnach commented 2018-10-02 12:08:45 -05:00 (Migrated from github.com)

@kinggoesgaming I think a newtype isn't a natural solution here. It's really the same type, just serialized in a different way, and it may even depend on the context what serialization to use. I like the suggestion to use free functions for use with serde attributes, since it makes the intention of client code immediately obvious.

@kinggoesgaming I think a newtype isn't a natural solution here. It's really the same type, just serialized in a different way, and it may even depend on the context what serialization to use. I like the suggestion to use free functions for use with serde attributes, since it makes the intention of client code immediately obvious.
Redrield commented 2018-10-02 13:03:25 -05:00 (Migrated from github.com)

I'm thinking add a module that can be used with #[serde(with = "uuid::dense_serde")]. (I'm not sure about the name, just an example)

I'm thinking add a module that can be used with `#[serde(with = "uuid::dense_serde")]`. (I'm not sure about the name, just an example)
👍 1
KodrAus commented 2018-10-02 18:06:34 -05:00 (Migrated from github.com)

What do you think of uuid::serde::compact? That way we've got a serde module too that any other serde specific stuff can live in?

What do you think of `uuid::serde::compact`? That way we've got a `serde` module too that any other `serde` specific stuff can live in?
kinggoesgaming commented 2018-10-02 18:15:50 -05:00 (Migrated from github.com)

note uuid::serde namespace can't be used because it conflicts with serde crate

**note** `uuid::serde` namespace can't be used because it conflicts with `serde` crate
KodrAus commented 2018-10-03 00:01:08 -05:00 (Migrated from github.com)

Ok, so we could do uuid::adapter::compact, or uuid::adapter::dense instead ?

Ok, so we could do `uuid::adapter::compact`, or `uuid::adapter::dense` instead ?
Redrield commented 2018-10-03 08:17:32 -05:00 (Migrated from github.com)

Rewritten to use serde attributes for dense serialization under uuid::adapter::compact

Rewritten to use serde attributes for dense serialization under uuid::adapter::compact
Dylan-DPC-zz (Migrated from github.com) requested changes 2018-10-03 08:35:37 -05:00
src/adapter/compact.rs
@ -0,0 +1,117 @@
//! Module for use with `#[serde(with = "...")]` to serialize a [`Uuid`]
Dylan-DPC-zz (Migrated from github.com) commented 2018-10-03 08:33:25 -05:00

uhmm?

uhmm?
src/adapter/compact.rs
@ -0,0 +33,4 @@
pub fn deserialize<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Uuid, D::Error> {
struct DenseUuidBytesVisitor;
Dylan-DPC-zz (Migrated from github.com) commented 2018-10-03 08:34:32 -05:00

do we need the struct inside of the function? Generally not a fan of this.

do we need the struct inside of the function? Generally not a fan of this.
Redrield (Migrated from github.com) reviewed 2018-10-03 08:38:06 -05:00
src/adapter/compact.rs
@ -0,0 +1,117 @@
//! Module for use with `#[serde(with = "...")]` to serialize a [`Uuid`]
Redrield (Migrated from github.com) commented 2018-10-03 08:38:05 -05:00

Debug messages, i'll take those out, whoops!

Debug messages, i'll take those out, whoops!
Redrield (Migrated from github.com) reviewed 2018-10-03 08:38:31 -05:00
src/adapter/compact.rs
@ -0,0 +33,4 @@
pub fn deserialize<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Uuid, D::Error> {
struct DenseUuidBytesVisitor;
Redrield (Migrated from github.com) commented 2018-10-03 08:38:31 -05:00

I was following in the pattern of the String visitors in the Deserialize impl in serde_support.rs, it can be moved if you prefer

I was following in the pattern of the String visitors in the Deserialize impl in serde_support.rs, it can be moved if you prefer
Dylan-DPC-zz (Migrated from github.com) reviewed 2018-10-03 08:40:51 -05:00
src/adapter/compact.rs
@ -0,0 +33,4 @@
pub fn deserialize<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Uuid, D::Error> {
struct DenseUuidBytesVisitor;
Dylan-DPC-zz (Migrated from github.com) commented 2018-10-03 08:40:51 -05:00

It is fine. Just something I don't like personally but i'm okay with it if it keeps it consistent with other impls in the crate

It is fine. Just something I don't like personally but i'm okay with it if it keeps it consistent with other impls in the crate
Dylan-DPC-zz (Migrated from github.com) approved these changes 2018-10-03 08:41:14 -05:00
kinggoesgaming (Migrated from github.com) reviewed 2018-10-03 10:26:14 -05:00
src/adapter/compact.rs
@ -0,0 +1,117 @@
//! Module for use with `#[serde(with = "...")]` to serialize a [`Uuid`]
kinggoesgaming (Migrated from github.com) commented 2018-10-03 10:26:14 -05:00

add a // TODO just to be sure :)

we have a bot that marks TODO comments

add a // TODO just to be sure :) we have a bot that marks TODO comments
Redrield (Migrated from github.com) reviewed 2018-10-03 10:32:41 -05:00
src/adapter/compact.rs
@ -0,0 +1,117 @@
//! Module for use with `#[serde(with = "...")]` to serialize a [`Uuid`]
Redrield (Migrated from github.com) commented 2018-10-03 10:32:41 -05:00

wdym? Removed them in b1b2efe

wdym? Removed them in b1b2efe
kinggoesgaming (Migrated from github.com) approved these changes 2018-10-03 11:14:49 -05:00
kinggoesgaming commented 2018-10-03 11:15:06 -05:00 (Migrated from github.com)

bors r+

bors r+
bors[bot] commented 2018-10-03 11:22:42 -05:00 (Migrated from github.com)

Build succeeded

# Build succeeded * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/436719190?utm_source=github_status&utm_medium=notification) * [continuous-integration/appveyor/branch](https://ci.appveyor.com/project/KodrAus/uuid/build/1.0.1016)
kinggoesgaming (Migrated from github.com) reviewed 2019-01-28 14:32:45 -06:00
src/adapter/compact.rs
@ -0,0 +33,4 @@
pub fn deserialize<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Uuid, D::Error> {
struct DenseUuidBytesVisitor;
kinggoesgaming (Migrated from github.com) commented 2019-01-28 14:32:44 -06:00

Huuuuuuggggeee necro...

It just keeps it very much contained and prevents any accidental use outside the function

Huuuuuuggggeee necro... It just keeps it very much contained and prevents any accidental use outside the function
Sign in to join this conversation.
Reviewers
No reviewers
radix
KodrAus
Dylan-DPC-zz
kinggoesgaming
Labels
Clear labels   
breaking
This causes a breaking change and will be merged to breaking branch.

  
bug

   
dependencies
Pull requests that update a dependency file

  
good first issue

   
help wanted

   
unstable
Tracking issue for an unstable optional dependency

No labels
breaking
bug
dependencies
good first issue
help wanted
unstable
Milestone
Clear milestone
No items
No milestone
0.7.2
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-uuid-rs/uuid!331
No description provided.