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

Add "uuid" format #715

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Add "uuid" format #715

merged 1 commit into from
Mar 18, 2019

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Feb 23, 2019

Fixes #542.

@handrews
Copy link
Contributor Author

(presumably, the CI build will still be segfaulting)

@Julian
Copy link
Member

Julian commented Feb 24, 2019

I haven't gone and re-read RFC4122, but from experience a lot of places "argue" about emitting UUIDs with or without hyphens.

IIRC the RFC says to include them in the BNF, but to me it's almost common enough to specifically call out if we intend without-hyphen to be invalid (or vice versa if we intend them to be valid).

@handrews
Copy link
Contributor Author

@Julian thanks, good point, I don't know anything about that. Maybe I will split the "duration" change out of this so we can leave this open longer for feedback since there is a reasonable question here.

@handrews handrews changed the title "duration" and "uuid" formats Add "uuid" format Feb 24, 2019
@handrews
Copy link
Contributor Author

handrews commented Mar 4, 2019

@Julian my inclination is to start with saying that this format indicates / optionally validates that the string strictly conforms with the ABNF in the RFC (which does include the hyphens). It's easier to relax a constraint later than add one.

Alternatively, if anyone thinks this is likely to cause problems, we can punt this until the next draft which will presumably focus on format a lot anyway. I don't consider this as important as adding duration or fixing the reference for hostname.

@Julian
Copy link
Member

Julian commented Mar 4, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants