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

fix(specs): make languages an enum #2865

Merged
merged 5 commits into from Mar 18, 2024
Merged

fix(specs): make languages an enum #2865

merged 5 commits into from Mar 18, 2024

Conversation

kai687
Copy link
Contributor

@kai687 kai687 commented Mar 15, 2024

🧭 What and Why

Restrict allowed languages for settings like queryLanguages and indexLanguages to the list of supported languages, as currently the clients suggest that all string values are accepted, which would lead to errors when using.

Changes included:

  • Introduce a new enum supportedLanguage
  • Restrict items for indexLanguages, queryLanguages, removeStopWords, ignorePlurals, and the dictionary response to be of supportedLanguage type.

🧪 Test

  • Kotlin client fails to build.
  • Swift tests fail with some type error I don't quite understand.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Mar 15, 2024

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

@kai687 kai687 marked this pull request as ready for review March 15, 2024 11:11
@kai687 kai687 requested a review from a team as a code owner March 15, 2024 11:11
@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

Inre-run the Kotlin one to check it wasn't just a one-off error
The Swift error is interesting, let me take a look in the generated code

@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

It seems kotlin doesn't understand the oneOf anymore, now that there is a $ref in one of the element...
I can also take a look

@kai687
Copy link
Contributor Author

kai687 commented Mar 15, 2024

Sorry about the additional work and thanks.

@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

I found the issue for Swift, I'm gonna push a fix

@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

I found the issue for Kotlin, I'm gonna try to push a fix 😄

@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

@aallam This commit fixes the issue, but I would love to have you to take a look at it next week, to confirm

@Fluf22
Copy link
Contributor

Fluf22 commented Mar 15, 2024

@kai687 All good, everything is fixed for now, you can merge!
@aallam will take a look next week, and we will push a fix in another PR if needed.

Copy link

@github-actions github-actions bot temporarily deployed to pull request March 15, 2024 17:13 Inactive
Copy link
Contributor

@Fluf22 Fluf22 left a comment

Choose a reason for hiding this comment

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

Nice 🆗

@kai687
Copy link
Contributor Author

kai687 commented Mar 18, 2024

@Fluf22 thanks for doing this! I don't have permissions to merge PRs on this repo. Could you merge it for me please?

@Fluf22 Fluf22 merged commit 7dc719c into main Mar 18, 2024
24 checks passed
@Fluf22 Fluf22 deleted the fix/language-is-enum branch March 18, 2024 10:18
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

3 participants