-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(clients): add Kotlin API client #1400
Conversation
✅ Deploy Preview for api-clients-automation ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
a1b05fa
to
ae4ae98
Compare
d5d1dc6
to
899db6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks really nice ! First pass on the form of the PR, there is too much to look at everything ahah, GG !
...iasearch-client-kotlin/client/src/commonMain/kotlin/com/algolia/client/configuration/Host.kt
Outdated
Show resolved
Hide resolved
...algoliasearch-client-kotlin/client/src/commonMain/kotlin/com/algolia/client/api/SearchApi.kt
Outdated
Show resolved
Hide resolved
tests/output/java/bin/test/com/algolia/methods/requests/SearchClientRequestsTests.class
Outdated
Show resolved
Hide resolved
e0cc2a1
to
a68c7a3
Compare
47d7508
to
fe26fa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing work !
...s/algoliasearch-client-kotlin/client/src/commonMain/kotlin/com/algolia/client/BuildConfig.kt
Outdated
Show resolved
Hide resolved
...-kotlin/client/src/commonMain/kotlin/com/algolia/client/configuration/internal/HttpClient.kt
Show resolved
Hide resolved
generators/src/main/java/com/algolia/codegen/AlgoliaKotlinGenerator.java
Show resolved
Hide resolved
generators/src/main/java/com/algolia/codegen/cts/AlgoliaCTSGenerator.java
Outdated
Show resolved
Hide resolved
4edd2c9
to
f95ce1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice !
I think there is an issue with the CI, there is no code generated, can you try to bump the cache key pls ?
ah maybe it's not a cache key issue but a artifacts issue, we use them to pass file from jobs to jobs in the CI, you need to add one for kotlin in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question following up @millotp remarks about versioning!
Otherwise, it looks really great!! Amazing work 🙌🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks super clean, gg!
...client-kotlin/client/src/commonMain/kotlin/com/algolia/client/configuration/Configuration.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to add on my side, SHIP IT!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually can you bump this value? https://github.com/algolia/api-clients-automation/blob/main/.github/.cache_version
so we get a full CI run
|
||
# Kotlin | ||
- name: Download clients-kotlin artifact | ||
if: ${{ inputs.kotlin == 'true' && inputs.type == 'all' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add kotlin: true
in main.yml
where we call restore-artifacts pls ? otherwise this is ignored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm sorry I didn't except the CI to be that complicated, maybe it's easier if you make it work in another PR, this one is already huge and awesome, let's start with that !
To make the CI push the generated code you need to edit the setRunVariables.ts
file to export a RUN_GEN_KOTLIN
var
@millotp Hopefully this time is the one 😅 otherwise I will make another PR for the CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like there is still something missing, but I'm not sure what
algolia/api-clients-automation#1400 Co-authored-by: Mouaad Aallam <Mouaad@Aallam.com>
🧭 What and Why
Generation of Kotlin API Client with the following specification:
kotlinx.serialization
for serialization.🎟 JIRA Ticket:
APIC-651
APIC-652
Changes included: