API Nits/general "I had trouble with this"

[DianaNites]

As requested, from my thread on twitter, and a few more I just ran into.

The API is kind of confusing to me, in particular:

---

Lack of proper little-endian support, which was partially removed?

This also plays into my confusion, theres Uuid::(to/from)_fields_le and Uuid::(to/from)_u128_le, but not Uuid::from_bytes_le? Theres Uuid::as_bytes but not Uuid::as_bytes_le?

This in particular is complicating things for me, as the Uuids I want to handle are "inspired" by Microsoft and stored mixed-endian. (Also, something I encountered when switching the bytes myself is Uuid::get_version_num returning 14 instead of 4, so those specific last few bits may need special handling?)

See #462

---

The crate supports no_std, but the Uuid type isn't FFI safe/repr(transparent)? There doesn't seem to be any reason for it not to be. That attribute is stable since 1.28.0, which is below the crates MSRV.

See #463

---

API naming is kind of weird and inconsistent, Uuid is Copy but Uuid::as_bytes exists? Why not Uuid::into_bytes, theres no Uuid::as_mut_bytes anyway? This also matches the Rust API Guidelines C-CONV.

See #465

---

The (from/to/as?)_u128 api is confusing, because Uuids aren't u128s at all! If someone has a u128 they want to make a Uuid, they can always extract a byte slice from it and use Uuid::from_bytes(_le)? Though, granted, the std methods to go from integers to bytes weren't stabilized until 1.32.0, which is one version above the MSRV. (Also the documented MSRV is 1.31.0 but travis CI is pinned to 1.32.0, even for 0.8.1?)

Uuid::get_version and Uuid::get_version_num are kind of confusing?, especially since the latters documentation says only V4 is supported? I think it'd be better if Uuid::get_version returned just Version, and instead Version had a Unknown(usize) variant, though that might technically make adding a new recognized version would be breaking, since it wouldn't match Unknown(usize) anymore?

It's unclear to me why all the to_*_ref APIs exist, given that Uuid is Copy?

Most of the methods returning Result probably.. shouldnt?

from_slice probably doesn't need to exist anymore if you're going to be handling #457, as there are impls for arrays up to length 32 in std, so it can be replaced with Uuid::from_bytes(slice.try_into().unwrap())

from_fields(_le) should either take a fixed-size array, same reason as above.

new_v1 should take an array, same reasons as above. Then it doesn't need a Result, as that and from_fields(_le) are the only reason it does.

from_guid even without any of the above shouldn't leak a Result? GUID::Data4 is an array, so the from_fields_le call can never be an error?

Just ran into this one, but Uuid::new_v4 says it uses the rand crates default RNG, and that you can use Rand trait if you need a custom generator, but there doesn't actually seem to be a way to give Uuid your own random bytes? Something like with_v4_random?

On that note, my crate is no_std and i'd like to use V4 Uuids, so new_v4 should probably be gated on the std feature, and with_v4_random not.

Speaking of v4, naming: Would it perhaps make more sense for Uuid::nil to be renamed Uuid::new and the new_vX methods renamed to with_vX? I think that'd make more sense and be more inline with the Rust API Guidelines?

Also, Default is manually implemented but it could be derived? Fixed size arrays(under length 32) implement Default, and Uuid/[u8; 16] fit the bill.

Also, the MSRV should probably be mentioned in the README more explicitly. The badge is easy to miss, and I did miss it at first.

See #464

---

Sorry for the long issue and if it seems like i'm bashing the library :( just what I struggled with when using the library, and apologies if some of these have been addressed on the master branch?

[DianaNites]

Just remembered serde was a thing so also that Uuid, a fixed size array, is serialized as a slice, issue #329, and it really probably should become the default instead of needing the compact adapter?

[jamesmunns]

I'd like to add that I got bit by the "v4" feature requiring std. It would be nice to have UUIDv4 by providing and RNG or just creating directly with bytes.

[KodrAus]

Ah, we do actually support creating random uuids without needing the v4 feature using a Builder:

// Some external source of randomness that gives a `[u8; 16]`
let random_bytes = rng();

let uuid = Builder::from_bytes(random_bytes)
    .set_variant(Variant::RFC4122)
    .set_version(Version::Random)
    .build();

We could probably call this out in the docs for new_v4 better.

[KodrAus]

I've spent some time on this and I think we've fixed up a lot of these papercuts now. There are links in the issues that were split from this one, but we now:

• Have much better docs on endianness
• Support parsing mixed-endian buffers
• Serialize as a 16-element tuple in serde by default
• Have a Builder::from_random_bytes method that makes creating a V4 UUID without getrandom easier
• Fixed up lots of other nits and issues in the docs

I'll go ahead and close this one now. Any new issues or papercuts that come up in the future would be great to track in new issues. Thanks again for all the awesome feedback! It's helped make this library much more useful.