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

Adding a UUID format #546

Merged
merged 2 commits into from Apr 19, 2022
Merged

Adding a UUID format #546

merged 2 commits into from Apr 19, 2022

Conversation

chrusty
Copy link
Contributor

@chrusty chrusty commented Apr 7, 2022

This introduces a UUID format, using github.com/google/uuid to perform validation.

@jxsl13
Copy link

jxsl13 commented Apr 7, 2022

nice

@deepmap-marcinr deepmap-marcinr merged commit aeb264f into deepmap:master Apr 19, 2022
rliebz added a commit to rliebz/oapi-codegen that referenced this pull request Apr 21, 2022
With deepmap#546 merged in, UUIDs are defined as an alias for a string.

Before that implementation gets mass adoption, I wanted to propose a
slightly different alternative: basing the custom type off of
`uuid.UUID` over `string`.

This has the advantages of:
- Being convertable back to a `uuid.UUID` without having to parse the
  string a second time, or opening the conversion up to failure
  scenarios
- Still being easily convertable to `string` with the `uuid.UUID`'s
  `String` method
- Taking up less space in memory
- Reducing the number of invalid representations, since only `[16]byte`s
  can be represented, as opposed to all strings
- Not adding any dependencies, since `uuid.UUID` is already being
  imported
deepmap-marcinr pushed a commit that referenced this pull request May 4, 2022
With #546 merged in, UUIDs are defined as an alias for a string.

Before that implementation gets mass adoption, I wanted to propose a
slightly different alternative: basing the custom type off of
`uuid.UUID` over `string`.

This has the advantages of:
- Being convertable back to a `uuid.UUID` without having to parse the
  string a second time, or opening the conversion up to failure
  scenarios
- Still being easily convertable to `string` with the `uuid.UUID`'s
  `String` method
- Taking up less space in memory
- Reducing the number of invalid representations, since only `[16]byte`s
  can be represented, as opposed to all strings
- Not adding any dependencies, since `uuid.UUID` is already being
  imported
hikhvar pushed a commit to hikhvar/oapi-codegen that referenced this pull request Jun 3, 2022
We make heavy use of the new UUID feature introduced in deepmap#571, deepmap#546 and deepmap#556.
However, we observed that using a UUID as parameter creates malformed
strings in the requests made by the generated clients. This adds a
special handling for types.UUID in the StyleParamWithLocation function.
I'm not that happy with this approach. I would like to use an interface
"MarshalParameter" for this with semantics similar to MarshalJSON. But
that doesn't work for aliased types as UUIDs.
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
* Adding a UUID format

* Removing indirect (hopefully build passes now)

Co-authored-by: Chrusty <>
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
With deepmap#546 merged in, UUIDs are defined as an alias for a string.

Before that implementation gets mass adoption, I wanted to propose a
slightly different alternative: basing the custom type off of
`uuid.UUID` over `string`.

This has the advantages of:
- Being convertable back to a `uuid.UUID` without having to parse the
  string a second time, or opening the conversion up to failure
  scenarios
- Still being easily convertable to `string` with the `uuid.UUID`'s
  `String` method
- Taking up less space in memory
- Reducing the number of invalid representations, since only `[16]byte`s
  can be represented, as opposed to all strings
- Not adding any dependencies, since `uuid.UUID` is already being
  imported
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

Successfully merging this pull request may close these issues.

None yet

3 participants