gh-uuid-rs/uuid

Watch

Fork 0

Allow Uuid v3 and v5 be constructed from bytes #270

Merged

Marwes merged 6 commits from patch-1 into breaking

2018-07-07 13:02:36 -05:00

Marwes commented

2018-07-02 11:00:30 -05:00

(Migrated from github.com)

I'm submitting a ...

bug fix
feature enhancement
deprecation or removal
refactor

Description

Allow byte slices to be used for uuid construction

Motivation

I don't believe there is anything that specifies that these should be created from utf-8 encoded
strings so expanding these to conver byte slices as well should be harmless
(might be a breaking change though due to inference changes).

Tests

N/A

https://github.com/uuid-rs/uuid/issues/271

**I'm submitting a ...** - [ ] bug fix - [x] feature enhancement - [ ] deprecation or removal - [ ] refactor # Description Allow byte slices to be used for uuid construction # Motivation I don't believe there is anything that specifies that these should be created from utf-8 encoded strings so expanding these to conver byte slices as well should be harmless (might be a breaking change though due to inference changes). # Tests N/A # Related Issue(s) https://github.com/uuid-rs/uuid/issues/271

radix (Migrated from github.com) reviewed

2018-07-02 11:00:30 -05:00

KodrAus (Migrated from github.com) reviewed

2018-07-02 11:00:30 -05:00

GitCop commented

2018-07-02 11:00:33 -05:00

(Migrated from github.com)

There were the following issues with your Pull Request

Commit: be479fa94bcecb4a0bd02d89bfa231c5930906cb
Your commit message body contains a line that is longer than 72 characters
Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/uuid-rs/uuid

This message was auto-generated by https://gitcop.com

There were the following issues with your Pull Request * Commit: be479fa94bcecb4a0bd02d89bfa231c5930906cb * Your commit message body contains a line that is longer than 72 characters * Commits must be in the following format: **%{type}(%{scope}): %{description}** Guidelines are available at https://github.com/uuid-rs/uuid --- This message was auto-generated by https://gitcop.com

Dylan-DPC-zz commented

2018-07-02 13:11:59 -05:00

(Migrated from github.com)

hi @Marwes thanks for the PR.

We have started working on a 0.7.0 already so it will be better to target the breaking branch instead of master.

hi @Marwes thanks for the PR. We have started working on a 0.7.0 already so it will be better to target the breaking branch instead of master.

👍 1

GitCop commented

2018-07-03 03:50:32 -05:00

(Migrated from github.com)

There were the following issues with your Pull Request

Commit: 8862f25a896f12abbd0363cb59a6eea399900a7e
Your commit message body contains a line that is longer than 72 characters
Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/uuid-rs/uuid

This message was auto-generated by https://gitcop.com

There were the following issues with your Pull Request * Commit: 8862f25a896f12abbd0363cb59a6eea399900a7e * Your commit message body contains a line that is longer than 72 characters * Commits must be in the following format: **%{type}(%{scope}): %{description}** Guidelines are available at https://github.com/uuid-rs/uuid --- This message was auto-generated by https://gitcop.com

GitCop commented

2018-07-03 03:58:56 -05:00

(Migrated from github.com)

There were the following issues with your Pull Request

Commit: e6574fcf8e
Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/uuid-rs/uuid

This message was auto-generated by https://gitcop.com

There were the following issues with your Pull Request * Commit: e6574fcf8e5646787b9b93e3f69cb68652b64d30 * Commits must be in the following format: **%{type}(%{scope}): %{description}** Guidelines are available at https://github.com/uuid-rs/uuid --- This message was auto-generated by https://gitcop.com

kinggoesgaming commented

2018-07-03 07:28:57 -05:00

(Migrated from github.com)

Ignore gitcop for now

kinggoesgaming commented

2018-07-03 10:00:35 -05:00

(Migrated from github.com)

@Marwes run cargo fmt with the latest nightly to fix build errors

GitCop commented

2018-07-03 10:29:03 -05:00

(Migrated from github.com)

There were the following issues with your Pull Request

Commit: e6574fcf8e
Commits must be in the following format: %{type}(%{scope}): %{description}
Commit: af5fca9379
Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/uuid-rs/uuid

This message was auto-generated by https://gitcop.com

There were the following issues with your Pull Request * Commit: e6574fcf8e5646787b9b93e3f69cb68652b64d30 * Commits must be in the following format: **%{type}(%{scope}): %{description}** * Commit: af5fca9379ed0f28aad4a7c86882b4fa6542a676 * Commits must be in the following format: **%{type}(%{scope}): %{description}** Guidelines are available at https://github.com/uuid-rs/uuid --- This message was auto-generated by https://gitcop.com

kinggoesgaming commented

2018-07-03 11:34:03 -05:00

(Migrated from github.com)

Thinking about this again... I don't think this should be considered a breaking change, as the change only makes input more permissible, not less.

Marwes commented

2018-07-03 14:35:10 -05:00

(Migrated from github.com)

Thinking about this again... I don't think this should be considered a breaking change, as the change only makes input more permissible, not less.

It can break things like new_v5(String::new().as_ref()), which are easy to fix but can still be annoying. rust does allow for such breakages from the standard library but since there is actually a breaking release planned here I think it is safer to just wait.

> Thinking about this again... I don't think this should be considered a breaking change, as the change only makes input more permissible, not less. It can break things like `new_v5(String::new().as_ref())`, which are easy to fix but can still be annoying. rust does allow for such breakages from the standard library but since there is actually a breaking release planned here I think it is safer to just wait.

kinggoesgaming (Migrated from github.com) reviewed

2018-07-06 21:12:25 -05:00

kinggoesgaming (Migrated from github.com) left a comment

No description provided.

Other than removing the `cfg` attributes: why do we now have a `new_vx` and a `new_vx_` functions?

Marwes commented

2018-07-07 02:39:54 -05:00

(Migrated from github.com)

Other than removing the cfg attributes: why do we now have a new_vx and a new_vx_ functions?

It ensures that the actual implementation only gets monomorphized once, regardless of the type of S. Alternatively we could make the new_ functions only take &[u8]

> Other than removing the cfg attributes: why do we now have a new_vx and a new_vx_ functions? It ensures that the actual implementation only gets monomorphized once, regardless of the type of `S`. Alternatively we could make the `new_` functions only take `&[u8]`

Dylan-DPC-zz commented

2018-07-07 09:03:29 -05:00

(Migrated from github.com)

@Marwes Since we are okay with a breaking change, i think it is better to combine both functions into a single function.

Marwes commented

2018-07-07 09:09:46 -05:00

(Migrated from github.com)

@Dylan-DPC Changed the functions to only take &[u8]

@Dylan-DPC Changed the functions to only take `&[u8]`

👍 2

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

2018-07-07 12:56:33 -05:00

Dylan-DPC-zz commented

2018-07-07 12:56:42 -05:00

(Migrated from github.com)

bors: r+

bors[bot] commented

2018-07-07 13:02:36 -05:00

(Migrated from github.com)

Build succeeded

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