-
Notifications
You must be signed in to change notification settings - Fork 1
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 check on language files and add missing translations for en and es files #999
Conversation
@KillianCourvoisier can you review the new Spanish translations? |
5696e6d
to
1bd78f7
Compare
white_label/configure-brand.ts
Outdated
|
||
const areExtraFrEquivalent = areExtraLanguagesEquivalent('en') | ||
const areExtraEnEquivalent = areExtraLanguagesEquivalent('en') | ||
const areExtraEsEquivalent = areExtraLanguagesEquivalent('fr') |
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.
en en fr
where as it should be fr en es
? Or I did not understand?
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.
const extraLanguages = ['fr', 'en', 'es']
const areExtraLangagesOk = extraLanguages.every(extraLanguage => areExtraLanguagesEquivalent(extraLanguage))
seems just as clear and less prone to error for me
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 catch, i fixed it here
Some considerations:
- I applied your suggestion also for
areLanguagesEquivalent
calls - This implies that there will be a useless call to
areLanguagesEquivalent('en', 'en')
, comparing english to itself- I consider this not a problem as the result will be always be
true
and this is fast enough to not be perceived. Is is acceptable considering the improvement in code readibility
- I consider this not a problem as the result will be always be
- I could have put
areLanguagesEquivalent
andareExtraLanguagesEquivalent
in the same loop, but I prefer it this way so the console prints will be grouped by file type (languages, then extra-languages)
1bd78f7
to
2bd5d7d
Compare
Some languages files were missing translations and also extra languages were not in sync with each other in white labels Those new checks should ensure that all langages files contain the same keys and that all extra language override the same keys
2bd5d7d
to
5e54766
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.
LGTM 👍
Some languages files were missing translations and also extra languages were not in sync with each other in white labels
Those new checks should ensure that all langages files contain the same keys and that all extra language override the same keys