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: add missing languages and correctly find languageCode #27

Merged
merged 8 commits into from
Jan 29, 2022
Merged

fix: add missing languages and correctly find languageCode #27

merged 8 commits into from
Jan 29, 2022

Conversation

ImRodry
Copy link
Contributor

@ImRodry ImRodry commented Jan 28, 2022

This PR adds missing languages that were recently added to Crowdin (obtained through /languages on the API) and also fixes the find function to look for the osxLocale instead of the twoLettersCode, as this code is often repeated in some languages and caused them to be incorrectly retrieved. These changes were tested and confirmed to work. There is one issue left, which is that, for files that are only available to specific languages, the client will throw an error as it cannot fetch the endpoint. I can fix this by removing these files from the final array, let me know if you'd like that fix or a different approach, as the CDN currently does not tell us which files are available in what languages beforehand.

@ImRodry ImRodry changed the title chore: add missing languages fix: add missing languages and correctly find languageCode Jan 28, 2022
@ImRodry
Copy link
Contributor Author

ImRodry commented Jan 28, 2022

The last commit is a fix to projects with files hidden to certain languages, which simply catches the request and makes it fall back to null so it can later be removed from getLanguageTranslations

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@ImRodry
Copy link
Contributor Author

ImRodry commented Jan 29, 2022

I committed that fix a while ago but for some reason github didn’t pick up, so I force-pushed

src/index.ts Outdated Show resolved Hide resolved
@yevheniyJ
Copy link
Collaborator

Looks good. Just a minor comment. I will merged and release new version tomorrow and also will bump the axios library

@ImRodry
Copy link
Contributor Author

ImRodry commented Jan 29, 2022

Done. Please use the squash and merge option when you merge since we got a few commits here :P

@ImRodry
Copy link
Contributor Author

ImRodry commented Jan 29, 2022

BTW I plan on opening a second PR to fix a few performance issues and other minor improvements, if you wanna wait a few hours for that, otherwise feel free to merge and release and that can come later :)

@yevheniyJ yevheniyJ merged commit cea7a1a into crowdin:main Jan 29, 2022
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

2 participants