-
Notifications
You must be signed in to change notification settings - Fork 248
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
(refactor) O3-3522: Replace module-local translations with core translations #1895
Conversation
@jnsereko, @denniskigen, @ibacher, could you please help me review my PR please? Besides, I am encountering this error: Argument of type '"patientIdentifierSticker"' is not assignable to parameter of type '"address" | "male" | "female" and more additional strings that are not present in the esm-translations file that I believe I should add Kindly feel free to correct me if necessary. Thank you! |
Short version is not everything goes into Core Translations, only strings that we use in several places. |
@ibacher thanks for the review and guidance |
6fed1ec
to
16c0ed9
Compare
@ibacher I have fixed the proposed changes. |
…-chart into (refactor)O3-3522
@ibacher, would it be okay for me to close this PR? I have already fixed the frequently used strings in the core translations. The specific or less frequently used strings are to be translated directly using the t() function in the code where they are used, based on your comment above which makes me believe these changes are not needed? kindly correct me please! |
@jwnasambu We merged in the PR with changes. I think having standardized translations for printing, etc. makes sense. However, in this PR you will need to update the versions of the CLI and framework as outlined here for this code to be able to "see" those changes. |
…-chart into (refactor)O3-3522
@ibacher, I have tried out something here and would appreciate your guidance and review. The build failure is from the programs app, which is not related to this file. |
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. Thanks, @jwnasambu.
Requirements
Summary
I replaced the t() calls with getCoreTranslation() calls knowing that getCoreTranslation behaves similarly to t() and returns the translated text.
Screenshots
Related Issue
https://openmrs.atlassian.net/browse/O3-3522
Other