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

Support V4 Uuids from arbitrary random bytes #170

Closed
1 of 3 tasks
KodrAus opened this issue Mar 17, 2018 · 5 comments
Closed
1 of 3 tasks

Support V4 Uuids from arbitrary random bytes #170

KodrAus opened this issue Mar 17, 2018 · 5 comments
Assignees
Milestone

Comments

@KodrAus
Copy link
Member

KodrAus commented Mar 17, 2018

Note: for support questions, please use gitter or users forums. This
repository's issues are reserved for feature requests and bug reports.

I'm submitting a ...

  • bug report
  • feature request
  • support request => Please do not submit support request here, see note
    at the top of this template.

Description

I thought we could flesh out our solution to creating uuids using alternative Rngs without having to make rand a public dependency.

new_v4_bat_scooter_name_not_important

servo/servo#20306 suggested a new_v4_from_bytes method as a workaround to needing to make rand a public dependency. Following the naming convention for other constructors, it might look like:

fn new_v4_from_bytes(b: &[u8]) -> Result<Uuid, ParseError>;
let uuid = Uuid::new_v4_from_bytes(b).expect("invalid length");

It would be nice to use [u8; 16] as the source instead of &[u8] so we can make the method infallible. It would be a bit inconsistent to use fn new_v4_from_bytes(b: [u8; 16]) since we already have fn from_bytes(b: &[u8]). new_v4_from_uuid_bytes is a bit of a nasty name. So for now it could be:

fn new_v4_bat_scooter_name_not_important(b: [u8; 16]) -> Uuid;
let uuid = Uuid::new_v4_bat_scooter_name_not_important(b);

We could even go ahead and name it something like new_v4_from_rng_bytes([u8; 16]) and deprecate it once rand is stable in favour of Rand::rand.

Builder

Edit: Moved to a seperate issue #171

Related to: #153

cc: @nox @kinggoesgaming @Dylan-DPC

@kinggoesgaming
Copy link
Member

kinggoesgaming commented Mar 17, 2018

We could even go ahead and name it something like new_v4_from_rng_bytes([u8; 16]) and deprecate it once rand is stable in favour of Rand::rand.

I like this name, as it clearly gets the meaning across.

Edit: removed suggestion on using #[unstable] attribute.


As per the Builder I think I will give it some thought before giving any comments on it.

@nox
Copy link
Contributor

nox commented Mar 17, 2018

I would use random instead of rng in the name. What do you mean by #[unstable]?

@kinggoesgaming
Copy link
Member

wait nevermind, ignore that #[unstable] I sometimes forget that unstable is std specific attribute

@Dylan-DPC-zz
Copy link
Member

Dylan-DPC-zz commented Mar 17, 2018

Will give it a try today :)

Edit: Have moved Builder Pattern to a seperate issue #171

@Dylan-DPC-zz Dylan-DPC-zz self-assigned this Mar 17, 2018
This was referenced Mar 17, 2018
@kinggoesgaming kinggoesgaming added this to the 0.6.2 milestone Mar 19, 2018
bors bot added a commit that referenced this issue Mar 20, 2018
172: from_rng_bytes added r=Dylan-DPC a=Dylan-DPC

<!--
    As we are working towards a stable version of uuid, we require that you 
    open an issue, before submitting a pull request. If the pull request is 
    imcomplete, prepend the Title with WIP: 
-->

**I'm submitting a ...**
  - [ ] bug fix
  - [x] feature enhancement
  - [ ] deprecation or removal
  - [ ] refactor

# Description
<!-- Provide a summary of your changes in the Title above -->
As discussed in the issue, this method is a solution to avoid making `rand` a public dependency

# Motivation
To avoid making `rand` public dependency
<!-- Why is this change required -->

# Tests
<!-- How are these changes tested? -->
none, test added 

# Related Issue(s)
#170 
<!-- 
    As noted above, we require an issue for every PR. Please link to the issue
    here
-->
bors bot added a commit that referenced this issue Mar 20, 2018
172: from_rng_bytes added r=kinggoesgaming a=Dylan-DPC

<!--
    As we are working towards a stable version of uuid, we require that you 
    open an issue, before submitting a pull request. If the pull request is 
    imcomplete, prepend the Title with WIP: 
-->

**I'm submitting a ...**
  - [ ] bug fix
  - [x] feature enhancement
  - [ ] deprecation or removal
  - [ ] refactor

# Description
<!-- Provide a summary of your changes in the Title above -->
As discussed in the issue, this method is a solution to avoid making `rand` a public dependency

# Motivation
To avoid making `rand` public dependency
<!-- Why is this change required -->

# Tests
<!-- How are these changes tested? -->
none, test added 

# Related Issue(s)
#170 
<!-- 
    As noted above, we require an issue for every PR. Please link to the issue
    here
-->
@kinggoesgaming
Copy link
Member

done

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