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

Provide uuid macro without dependencies #566

merged 2 commits into from Nov 16, 2021


Copy link

@Nugine Nugine commented Nov 16, 2021

I'm submitting a feature


#556 (comment)

Related Issue(s)

resolves #556

Copy link

KodrAus commented Nov 16, 2021

cc @QnnOkabayashi

src/ Show resolved Hide resolved
Copy link

@QnnOkabayashi QnnOkabayashi left a comment

At this point, we should change the feature gate to something like "macro-diagnostics". I also personally think that it should be on by default and require opting-out, since nice error messages should be the default. Thoughts?

@Nugine Nugine marked this pull request as draft Nov 16, 2021
Copy link

KodrAus commented Nov 16, 2021

A macro-diagnostics feature sounds like a good one to me 👍 Ideally we would have it on by default, but unfortunately for a non-trivial subset of users any additional dependencies will be pushed back against. That's one of our main reasons for a non-proc-macro version, and is also what landed us with a fast-rng feature that pulls in rand. I think there's an alternative approach to default features though that we can take: recommend a set of features to enable at the top of the README and in the crate-level docs:

version = "1"
features = [
    "v4",                # allow generating random UUIDs
    "fast-rng",          # use a faster (but still sufficiently random) algorithm
    "macro-diagnostics", # support more detailed diagnostics in the `uuid!` macro

It's not on by default, but it's recommended by default.

@Nugine Nugine marked this pull request as ready for review Nov 16, 2021
@Nugine Nugine requested a review from KodrAus Nov 16, 2021
Copy link

@KodrAus KodrAus left a comment

Thanks @Nugine! This looks good to me. I just had one suggestion on a missed feature rename in the docs.

src/ Outdated Show resolved Hide resolved
@KodrAus KodrAus merged commit 89530db into uuid-rs:main Nov 16, 2021
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

Macro construction without syn
3 participants