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

Make UUID an alias of uuid.UUID #571

Merged
merged 1 commit into from May 14, 2022
Merged

Make UUID an alias of uuid.UUID #571

merged 1 commit into from May 14, 2022

Conversation

Sh4rK
Copy link
Contributor

@Sh4rK Sh4rK commented May 12, 2022

Instead of wrapping Google's uuid.UUID, and reimplementing the
JSON logic, simply make it an alias instead.

The current version already leaks the implementation detail, so I
don't see any reason to also wrap it, and make its usage harder.

Continuation of #546 and #556.

Instead of wrapping Google's `uuid.UUID`, and reimplementing the
JSON logic, simply make it an alias instead.

The current version already leaks the implementation detail, so I
don't see any reason to also wrap it, and make its usage harder.
@deepmap-marcinr deepmap-marcinr merged commit c15d7e7 into deepmap:master May 14, 2022
@Sh4rK Sh4rK deleted the uuid-alias branch May 15, 2022 00:13
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
Instead of wrapping Google's `uuid.UUID`, and reimplementing the
JSON logic, simply make it an alias instead.

The current version already leaks the implementation detail, so I
don't see any reason to also wrap it, and make its usage harder.
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

2 participants