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/search by language code and expose manifest timestamp #10

Conversation

joelfsreis
Copy link
Contributor

@joelfsreis joelfsreis commented Jan 21, 2021

Expose Manifest timestamp.

  • This could be useful to know when we should pull new translations or use the cached ones.

Search by language code.

  • When calling listLanguages() I have the English language as en-GB and en-US. With current implementation is only possible to get the default English translation (with the twoLetterCode = 'en')
  • When calling getTranslations() it fails because does not find a match for the different English languages.
  • Searching by locale as well allows getting different translations for the same language and working as expected.

Copy link
Collaborator

@yevheniyJ yevheniyJ left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I left few minor comments which are mainly related to missing unit tests. Please let me know if you will struggle with fixing them.

src/internal/util/exportPattern.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
@joelfsreis
Copy link
Contributor Author

@yevheniyJ thanks for the quick feedback. I incorporated the requested changes, let me know if they are okay. Cheers

Copy link
Collaborator

@yevheniyJ yevheniyJ left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@yevheniyJ
Copy link
Collaborator

I will release new version with these changes later on today.

@yevheniyJ yevheniyJ merged commit a58cfd2 into crowdin:main Jan 22, 2021
@yevheniyJ
Copy link
Collaborator

@joelfsreis v0.1.1 is now available and contains these changes

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.

Calling getTranslations() throws an error due to multiple English translations
2 participants