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

chore: update models in kotlin client generator #1373

Closed
wants to merge 3 commits into from

Conversation

amagyar-iohk
Copy link
Contributor

@amagyar-iohk amagyar-iohk commented Sep 23, 2024

Description:

Adds solution when OAS model generation doesn't handle oneOf well

Checklist:

  • My PR follows the contribution guidelines of this project
  • My PR is free of third-party dependencies that don't comply with the Allowlist
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked the PR title to follow the conventional commit specification

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Unit Test Results

101 files  ±0  101 suites  ±0   18m 40s ⏱️ - 1m 1s
869 tests ±0  861 ✅ +1  8 💤 ±0  0 ❌  - 1 
876 runs  ±0  868 ✅ +1  8 💤 ±0  0 ❌  - 1 

Results for commit 2ef9431. ± Comparison against base commit 95d328e.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Integration Test Results

20 files  ±0  20 suites  ±0   2s ⏱️ ±0s
45 tests ±0  45 ✅ ±0  0 💤 ±0  0 ❌ ±0 
71 runs  ±0  71 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 2ef9431. ± Comparison against base commit 95d328e.

♻️ This comment has been updated with latest results.

@amagyar-iohk amagyar-iohk marked this pull request as ready for review September 24, 2024 11:45
@amagyar-iohk amagyar-iohk requested a review from a team as a code owner September 24, 2024 11:45
Comment on lines +3 to +5
* Please note:
* This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* Do not edit this file manually.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file really auto-generated? there Is some commented out code here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was auto generated but then I added to openapi-generator to ignore it and added to git

@@ -15,3 +16,5 @@ src/main/kotlin/org/hyperledger/identus/client/models/CredentialSubject.kt
src/main/kotlin/org/hyperledger/identus/client/models/DateTimeParameter.kt
src/main/kotlin/org/hyperledger/identus/client/models/DidParameter.kt
src/main/kotlin/org/hyperledger/identus/client/models/VcVerificationParameter.kt

src/main/kotlin/org/hyperledger/identus/client/models/Json.kt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably not understanding something, but I thought initially that you will override Service(id, type, serviceEndpoint) type here, should not you also add it in this file as well?

If there's no `discriminator` you'll have to try to parse each type to retrieve the typed object.

```kotlin
class ServiceTypeAdapter : TypeAdapter<AnimalResponse>() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be class AnimalResponseAdapter? 🤔

out.jsonValue(Gson().toJson(value.value))
}

fun JsonTypeAdapter.oleole() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you've been using this while developing and forgot to remove

@shotexa
Copy link
Contributor

shotexa commented Sep 26, 2024

@amagyar-iohk Are we still going to merge this one?

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.

2 participants