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

dhall-openapi: support signed openapi integers #2126

Merged
merged 3 commits into from
Jan 13, 2021

Conversation

WillSewell
Copy link
Collaborator

@WillSewell WillSewell commented Jan 4, 2021

OpenAPI integer types are signed (see integer), but they are currently decoded into the Dhall Natural type, which is unsigned. This PR updates the behaviour to decode into Dhall Integers.

OpenAPI intgeger types are signed, but they are currently decoded
into the Dhall Natural type, which is unsigned. This PR updates
the bheaviour to decode into Dhall Integers.
@Gabriella439
Copy link
Collaborator

@WillSewell: Would it be possible to still use Natural when the OpenAPI type specifies a minimum of 0? I haven't checked how much work it would entail, so if it's prohibitive then don't worry about it

@WillSewell WillSewell force-pushed the support-openapi-signed-ints branch from bee31a1 to 77da9c3 Compare January 8, 2021 11:49
@WillSewell
Copy link
Collaborator Author

Good point - I've pushed a PoC. I still need to test it out though.

@WillSewell WillSewell force-pushed the support-openapi-signed-ints branch from 77da9c3 to 37937f9 Compare January 8, 2021 13:53
@WillSewell
Copy link
Collaborator Author

WillSewell commented Jan 12, 2021

@Gabriel439 I think the most recent commit is correct. Unfortunately the kubernetes OpenAPI spec does not use minimum, so all the integers in dhall-kubernetes are assumed to be Integers.

@Gabriella439 Gabriella439 merged commit d5fad78 into dhall-lang:master Jan 13, 2021
@Gabriella439
Copy link
Collaborator

@WillSewell: Thank you! Yeah, it's fine if the Natural support doesn't yet benefit dhall-kubernetes specifically. We can always ask upstream to fix their OpenAPI specification if people want Natural numbers

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.

2 participants