gh-uuid-rs/uuid

Watch

Fork 0

Add little-endian functions #330

Merged

Veykril merged 5 commits from master into master

2018-10-01 17:51:55 -05:00

Veykril commented

2018-10-01 13:31:31 -05:00

(Migrated from github.com)

I'm submitting a feature

Description

This PR adds little-endian constructors as well as a to_bytes_le and a to_fields_le method. I've also renamed the previous bytes methods and appended the be suffix for consistency while adding the previous function names as wrapper methods around those to not break things. All of this is personal preference of course so far so please point out changes you want to have with this.

Edit: I forgot to mention that the le methods use the to_* naming conventions instead of as_* since they aren't a free conversion and dont return a &Bytes

Motivation

Issue #277.

Tests

Tested via doc-tests

#277

**I'm submitting a feature** # Description This PR adds little-endian constructors as well as a `to_bytes_le` and a `to_fields_le` method. I've also renamed the previous bytes methods and appended the be suffix for consistency while adding the previous function names as wrapper methods around those to not break things. All of this is personal preference of course so far so please point out changes you want to have with this. Edit: I forgot to mention that the le methods use the `to_*` naming conventions instead of `as_*` since they aren't a free conversion and dont return a `&Bytes` # Motivation Issue #277. # Tests Tested via doc-tests # Related Issue(s) #277

radix (Migrated from github.com) reviewed

2018-10-01 13:31:31 -05:00

KodrAus (Migrated from github.com) reviewed

2018-10-01 13:31:31 -05:00

kinggoesgaming (Migrated from github.com) reviewed

2018-10-01 13:31:31 -05:00

Dylan-DPC-zz (Migrated from github.com) reviewed

2018-10-01 14:28:25 -05:00

Veykril (Migrated from github.com) reviewed

2018-10-01 14:31:32 -05:00

src/lib.rs

Because we still keep the internal big-endian representation(as said in https://github.com/uuid-rs/uuid/issues/277#issuecomment-403608673).

Dylan-DPC-zz (Migrated from github.com) reviewed

2018-10-01 14:44:51 -05:00

src/lib.rs

Ah that's fine. It was a bit odd when I read the code. 👍

Ah that's fine. It was a bit odd when I read the code. :+1:

Dylan-DPC-zz commented

2018-10-01 17:16:54 -05:00

(Migrated from github.com)

@Veykril once you are done, run cargo fmt using the latest nightly.

@Veykril once you are done, run `cargo fmt` using the latest nightly.

Dylan-DPC-zz commented

2018-10-01 17:22:28 -05:00

(Migrated from github.com)

@Veykril is this ready for review or you have more stuff to add? (can remove the WIP from the title)

Veykril commented

2018-10-01 17:24:53 -05:00

(Migrated from github.com)

Should be ready now I believe

👍 1

Dylan-DPC-zz (Migrated from github.com) approved these changes

2018-10-01 17:28:54 -05:00

Dylan-DPC-zz commented

2018-10-01 17:39:07 -05:00

(Migrated from github.com)

bors: r+

bors[bot] commented

2018-10-01 17:51:54 -05:00

(Migrated from github.com)

Build succeeded

# Build succeeded * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/435878519?utm_source=github_status&utm_medium=notification) * [continuous-integration/appveyor/branch](https://ci.appveyor.com/project/KodrAus/uuid/build/1.0.1001)

kinggoesgaming commented

2018-10-02 15:09:54 -05:00

(Migrated from github.com)

I realized after the merge that this really messes with serde support... We want to look alternates to this approach

I realized after the merge that this really messes with `serde` support... We want to look alternates to this approach

Veykril commented

2018-10-02 15:18:38 -05:00

(Migrated from github.com)

How exactly does it mess with serde? The internal representation didn't change so it shouldn't have any impact with serialization, or am I missing something?

kinggoesgaming commented

2018-10-02 15:28:31 -05:00

(Migrated from github.com)

The user might be using something like bincode and expecting the LE read AND LE write... but serde ofc doesnt do that... its a BE write

The user might be using something like `bincode` and expecting the LE read AND LE write... but serde ofc doesnt do that... its a BE write

kinggoesgaming commented

2018-10-02 15:30:57 -05:00

(Migrated from github.com)

@Veykril we are currently discussing this on gitter if you want to join in: http://gitter.im/uuid-rs/Lobby

uuid-rs/Lobby
Welcome to rust uuid crates channel | uuid src: https://github.com/uuid-rs/uuid doc: https://docs.rs/uuid | uuid_macro src: https://github.com/uuid-rs/uuid_macro docs: https://docs.rs/uuid_macro

@Veykril we are currently discussing this on gitter if you want to join in: http://gitter.im/uuid-rs/Lobby <blockquote><img src="https://avatars-01.gitter.im/group/iv/4/5a6f9e69d73408ce4f8abb8a" width="48" align="right"><div><strong><a href="http://gitter.im/uuid-rs/Lobby">uuid-rs/Lobby</a></strong></div><div>Welcome to rust uuid crates channel | uuid src: https://github.com/uuid-rs/uuid doc: https://docs.rs/uuid | uuid_macro src: https://github.com/uuid-rs/uuid_macro docs: https://docs.rs/uuid_macro</div></blockquote>

Dylan-DPC-zz commented

2018-10-02 17:08:42 -05:00

(Migrated from github.com)

So what we were discussing is:

I am thinking of introducing 2 newtypes that allow that... UuidBe and UuidLe*
or we can just have a From/Into impls...
the way i was thinking was use UuidBe and UuidLe as adapters instead
so conversion is done explicitly by the user
this way the internal layout of Uuid bytes is left as impl detail

So what we were discussing is: > I am thinking of introducing 2 newtypes that allow that... `UuidBe` and `UuidLe`* or we can just have a From/Into impls... the way i was thinking was use UuidBe and UuidLe as adapters instead so conversion is done explicitly by the user this way the internal layout of Uuid bytes is left as impl detail