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

[Kotlin] add BigDecimal, BigInteger support #8880

Merged
merged 2 commits into from
Apr 16, 2021

Conversation

codymikol
Copy link
Contributor

this allows the kotlin client generator to support
BigDecimal values

This was originally created by @kuFEAR in their own branch, but a PR wasn't made into this project.
I hope you don't mind me making the PR for you, I happen to need this for my own project.

PR checklist

  • Read the contribution guidelines.

  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.

  • Run the following to build the project and update samples:

    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    

    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.

  • File the PR against the correct branch: master, 5.1.x, 6.0.x

  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. - [x]

Kotlin

@jimschubert (2017/09) ❤️, @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03)

@4brunu
Copy link
Contributor

4brunu commented Mar 4, 2021

Hey, thanks for your contribution 👍
One question, does kotlin client from OpenAPI generates the types BigDecimal and BigInteger?
Thanks

@kuFEAR
Copy link
Contributor

kuFEAR commented Mar 4, 2021

it works if you add typeMappings.set(mapOf("double" to "BigDecimal")) in configuration

@4brunu
Copy link
Contributor

4brunu commented Mar 4, 2021

If I'm understanding what you are saying, it's possible to change the typeMappings?
I didn't knew this feature.

But by default it doesn't generate BigDecimal from what I understood.

If by default the generator doesn't generate BigDecimal, I'm not sure if this makes sense to add this adapters to the default configuration.
Because if you use a custom types, you can add custom adapters via Serializer.moshiBuilder.add(...)

Also this PR adds the BigDecimal and BigInteger adapters, but it doesn't add them to Serializer.moshiBuilder.

I'm not sure whats the right move here, let's here the other technical committee members.

@codymikol
Copy link
Contributor Author

If I'm understanding what you are saying, it's possible to change the typeMappings?
I didn't knew this feature.

But by default it doesn't generate BigDecimal from what I understood.

If by default the generator doesn't generate BigDecimal, I'm not sure if this makes sense to add this adapters to the default configuration.
Because if you use a custom types, you can add custom adapters via Serializer.moshiBuilder.add(...)

Also this PR adds the BigDecimal and BigInteger adapters, but it doesn't add them to Serializer.moshiBuilder.

I'm not sure whats the right move here, let's here the other technical committee members.

That was actually supposed to be in there, I let it slip through after rebasing, just pushed it in now. Thanks for catching that!

@wing328
Copy link
Member

wing328 commented Mar 24, 2021

@codymikol can you please resolve the merge conflicts when you've time?

@codymikol
Copy link
Contributor Author

@codymikol can you please resolve the merge conflicts when you've time?

I'll take a look tomorrow

this allows the kotlin client generator to support
BigDecimal values

Fixes OpenAPITools#7196
@codymikol
Copy link
Contributor Author

codymikol commented Apr 12, 2021

@wing328 sorry for the delay

EDIT: Actually it looks like the CI is failing with some unrelated error, is this a problem for everyone?

@wing328
Copy link
Member

wing328 commented Apr 16, 2021

Updated the samples via 093d63b. Let's see if all the tests pass.

@wing328 wing328 added this to the 5.1.1 milestone Apr 16, 2021
@wing328 wing328 merged commit b5dac42 into OpenAPITools:master Apr 16, 2021
@codymikol
Copy link
Contributor Author

Thanks wing 👍

@wing328 wing328 changed the title feat(adapter): add BigDecimal kotlin support [Kotlin] add BigDecimal, BigInteger support Apr 16, 2021
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.

None yet

4 participants