from_rng_bytes added #172

Dylan-DPC-zz · 2018-03-17T16:02:28Z

I'm submitting a ...

bug fix
feature enhancement
deprecation or removal
refactor

Description

As discussed in the issue, this method is a solution to avoid making rand a public dependency

Motivation

To avoid making rand public dependency

Tests

none, test added

Related Issue(s)

#170

kinggoesgaming · 2018-03-17T18:09:04Z

src/lib.rs

@@ -1804,6 +1831,20 @@ mod tests {
        assert_eq!(&b_in, b_out);
    }

+    #[test]
+    #[cfg(feature = "v4")]
+    fn test_from_rng_bytes(){


Add space before {, rustfmt is complaining

KodrAus · 2018-03-18T00:49:50Z

src/lib.rs

+    #[cfg(feature = "v4")]
+    pub fn from_rng_bytes(b: [u8; 16]) -> Uuid {
+        let mut uuid = Uuid { bytes: [0; 16] };
+        copy_memory(&mut uuid.bytes, &b);


We also need to call set_variant and set_version here since the random bytes probably won't have the right values:

let mut uuid = Uuid { bytes: [0; 16] }; copy_memory(&mut uuid.bytes, &b); uuid.set_variant(UuidVariant::RFC4122); uuid.set_version(UuidVersion::Random); uuid

KodrAus · 2018-03-18T01:06:19Z

🚲🏚: I think I also agree with @nox's suggestion of using from_random_bytes instead of from_rng_bytes, since they're not really random number generator bytes, they're just random bytes.

nox · 2018-03-18T08:15:16Z

Neither from_rng_bytes nor from_random_bytes convey that we are building a v4 UUID, and those names are inconsistent with Uuid::new_v4.

KodrAus · 2018-03-18T08:19:23Z

Hmm, I think Uuid::from_random_bytes(b) conveys that at least as well as rng.gen::<Uuid>() did.

name changed to from_random_bytes

nox · 2018-03-18T11:21:39Z

This PR should include a patch to Uuid::new_v4 to make it call the new method we are introducing.

nox · 2018-03-18T11:47:44Z

src/lib.rs

+    /// assert_eq!(expected_uuid, uuid);
+    /// ```
+    ///
+    #[cfg(feature = "v4")]


There is no reason to feature-gate the method at all.

We are only generating v4 uuids from this. So why have it on other features?

Because there is no reason to have to bring in rand if one does not need it. For example in Servo we won't use new_v4 at all (and thus the rand in uuid) because we need a better RNG.

`<[_]>::copy_from_slice` is a better replacement Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>

175: Delete copy_memory function r=Dylan-DPC a=kinggoesgaming  **I'm submitting a ...** - [ ] bug fix - [ ] feature enhancement - [x] deprecation or removal - [x] refactor # Description Replace `uuid::copy_memory` with `<[_]>::copy_from_slice` # Motivation `core` already provides a good optimized version of `slice` copying function, so we dont need to create our own duplicate code. # Tests Our tests and benchmarks are running good, other than one or two runs which suggest a 1-2 nanosecond slowdown. # Related Issue(s) #173

KodrAus

Thanks @Dylan-DPC! Just a few nitpicks.

KodrAus · 2018-03-19T22:09:46Z

src/lib.rs

+    ///
+    pub fn from_random_bytes(b: [u8; 16]) -> Uuid {
+        let mut uuid = Uuid { bytes: [0; 16] };
+        copy_memory(&mut uuid.bytes, &b);


This wil need to be rebased now since copy_memory is now gone.

KodrAus · 2018-03-19T22:12:05Z

src/lib.rs

+    /// Basic usage:
+    ///
+    /// ```
+    /// use uuid::Uuid;


I still think we should add an example that's a little more motivating for people using this function, I don't think the value of the bytes we pass in, nor the assertion, should be visible in the remdered docs.

I just copied it from the examples given for the other functions. Will change it accordingly.

Edit: It is common to display the assertion in rust examples. I have seen it in other crates as well and we are doing it for the other examples too.

KodrAus · 2018-03-19T22:12:27Z

src/lib.rs

@@ -685,6 +684,34 @@ impl Uuid {
        Uuid { bytes: b }
    }

+    /// Creates a v4 Uuid from rng bytes (ref issue #170)


Probably don't need to reference GitHub issues in the docs prose.

sure. Since we have so many confusing functions to generate a UUID wanted to leave a note on why do we need this one 😄 will remove it

name changed to from_random_bytes

issue number removed from docs example changed

…re/from-rng

kinggoesgaming · 2018-03-20T04:12:03Z

@KodrAus any further nits? Else can you approve?

KodrAus · 2018-03-20T04:45:20Z

src/lib.rs

+    /// ```
+    ///
+    pub fn from_random_bytes(b: [u8; 16]) -> Uuid {
+        let mut uuid = Uuid { bytes: [0; 16] };


We don't need to create a new empty slice here and copy b into it, we could just create a Uuid { bytes: b }.

KodrAus

Thanks @Dylan-DPC! Looks good to me!

Dylan-DPC-zz · 2018-03-20T21:30:08Z

bors: r+

172: from_rng_bytes added r=Dylan-DPC a=Dylan-DPC  **I'm submitting a ...** - [ ] bug fix - [x] feature enhancement - [ ] deprecation or removal - [ ] refactor # Description  As discussed in the issue, this method is a solution to avoid making `rand` a public dependency # Motivation To avoid making `rand` public dependency  # Tests  none, test added # Related Issue(s) #170

kinggoesgaming · 2018-03-20T21:45:21Z

bors: r+

172: from_rng_bytes added r=kinggoesgaming a=Dylan-DPC  **I'm submitting a ...** - [ ] bug fix - [x] feature enhancement - [ ] deprecation or removal - [ ] refactor # Description  As discussed in the issue, this method is a solution to avoid making `rand` a public dependency # Motivation To avoid making `rand` public dependency  # Tests  none, test added # Related Issue(s) #170

bors · 2018-03-20T21:53:11Z

Build succeeded

Dylan-DPC added 3 commits March 17, 2018 21:12

from_rng_bytes added

4b64f47

feature check added

be08da9

method made public

dcb98d9

Dylan-DPC-zz requested review from kinggoesgaming, KodrAus and radix as code owners March 17, 2018 16:02

Dylan-DPC-zz added the Type:Enhancement label Mar 17, 2018

Dylan-DPC-zz added this to the 0.6.2 milestone Mar 17, 2018

Dylan-DPC added 2 commits March 17, 2018 23:06

allow dead code removed

35e340b

after running cargo fmt

289963e

kinggoesgaming requested changes Mar 17, 2018

View reviewed changes

kinggoesgaming approved these changes Mar 17, 2018

View reviewed changes

KodrAus reviewed Mar 18, 2018

View reviewed changes

add set variant & version

e9d48ba

name changed to from_random_bytes

nox reviewed Mar 18, 2018

View reviewed changes

Dylan-DPC and others added 7 commits March 18, 2018 17:41

feature gate removed

bd30f89

reroute new_v4 to from_random_bytes

4909a19

delete copy_memory

3533e5d

`<[_]>::copy_from_slice` is a better replacement Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>

unwanted feature gate removed

3b96ad2

change expected value because variant & version are added

9c0a85f

Merge branch 'master' into feature/from-rng

afb86ae

KodrAus reviewed Mar 19, 2018

View reviewed changes

Dylan-DPC added 3 commits March 20, 2018 03:51

from_rng_bytes added

32c9664

feature check added

f3b37e6

method made public

7885711

Dylan-DPC added 10 commits March 20, 2018 03:51

allow dead code removed

6ff8bb9

after running cargo fmt

e29c219

add set variant & version

1ef7e1b

name changed to from_random_bytes

feature gate removed

e5fcba5

reroute new_v4 to from_random_bytes

2b2784e

unwanted feature gate removed

ae19a1b

change expected value because variant & version are added

b64c2e2

rebased to copy_from_slice

fe4b105

issue number removed from docs example changed

Merge branch 'feature/from-rng' of github.com:uuid-rs/uuid into featu…

b9169fd

…re/from-rng

docs changed back to old style

afc61e9

KodrAus reviewed Mar 20, 2018

View reviewed changes

uuid object created directly

d4c2963

KodrAus approved these changes Mar 20, 2018

View reviewed changes

kinggoesgaming changed the title ~~from_rng_bytes added~~ wip: from_rng_bytes added Mar 20, 2018

Dylan-DPC-zz changed the title ~~wip: from_rng_bytes added~~ [WIP] from_rng_bytes added Mar 20, 2018

kinggoesgaming changed the title ~~[WIP] from_rng_bytes added~~ from_rng_bytes added Mar 20, 2018

kinggoesgaming changed the title ~~from_rng_bytes added~~ WIP from_rng_bytes added Mar 20, 2018

kinggoesgaming changed the title ~~WIP from_rng_bytes added~~ from_rng_bytes added Mar 20, 2018

bors bot merged commit d4c2963 into master Mar 20, 2018

This was referenced Mar 25, 2018

Make uuid like i32,f64,u8 etc. Can create const value. #183

Closed

add uuid build macro_rules #184

Closed

kinggoesgaming deleted the feature/from-rng branch March 25, 2018 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

from_rng_bytes added #172

from_rng_bytes added #172

Dylan-DPC-zz commented Mar 17, 2018

kinggoesgaming Mar 17, 2018

KodrAus Mar 18, 2018

KodrAus commented Mar 18, 2018

nox commented Mar 18, 2018

KodrAus commented Mar 18, 2018

nox commented Mar 18, 2018

nox Mar 18, 2018

Dylan-DPC-zz Mar 18, 2018

nox Mar 18, 2018

Dylan-DPC-zz Mar 18, 2018

KodrAus left a comment

KodrAus Mar 19, 2018

KodrAus Mar 19, 2018

Dylan-DPC-zz Mar 19, 2018 •

edited

Loading

KodrAus Mar 19, 2018

Dylan-DPC-zz Mar 19, 2018

kinggoesgaming commented Mar 20, 2018

KodrAus Mar 20, 2018

KodrAus left a comment

Dylan-DPC-zz commented Mar 20, 2018

kinggoesgaming commented Mar 20, 2018

bors bot commented Mar 20, 2018

from_rng_bytes added #172

from_rng_bytes added #172

Conversation

Dylan-DPC-zz commented Mar 17, 2018

Description

Motivation

Tests

Related Issue(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KodrAus commented Mar 18, 2018

nox commented Mar 18, 2018

KodrAus commented Mar 18, 2018

nox commented Mar 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KodrAus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dylan-DPC-zz Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinggoesgaming commented Mar 20, 2018

Choose a reason for hiding this comment

KodrAus left a comment

Choose a reason for hiding this comment

Dylan-DPC-zz commented Mar 20, 2018

kinggoesgaming commented Mar 20, 2018

bors bot commented Mar 20, 2018

Build succeeded

Dylan-DPC-zz Mar 19, 2018 •

edited

Loading