fundamental-ngx icon indicating copy to clipboard operation
fundamental-ngx copied to clipboard

fix: remove transform-translations from precommit hook, prepare *.properties files for new translations

Open mikerodonnell89 opened this issue 6 months ago • 3 comments

part of #12692

EDIT: after a team discussion, we've decided the best course of action is to remove the pre-commit hook that runs the *.properties to *.ts translation script. Moving forward with this PR. Original problem described below

Came across an interesting problem with a translation issue. In the issue linked above, we had to make a change to existing translated text, rather than adding a new translation. The text had been previously translated by the sap translation service. So in order to trigger the translation service that we need an updated translation, I need to modify each *.properties file. Since I'm not qualified to attempt translation myself, I'll need to change each *.properties file to English. Our pre-commit hook would automatically convert this to the *.ts for each language, and each (previously correctly translated) language would now have this string in English while we wait for the updated translations from the translation service. A potential alternative would be to remove the translation task that triggers automatically during the pre-commit hook, and developers for our library must be aware that they’ll need to run this script locally when necessary when adding new translations. So we have a decision to make with two options:

  • potential short-term translation regressions and keep the pre-commit hook as-is, or
  • remove the pre-commit hook as I’ve done in this PR, but we will need to understand that, when the translations are provided, we’ll need to run the script to convert the new *.properties files to *.ts.

mikerodonnell89 avatar May 19 '25 15:05 mikerodonnell89

Deploy Preview for fundamental-ngx ready!

Name Link
Latest commit 9afef0a1f33853643bf8e36fe76aabd0dc4d76e4
Latest deploy log https://app.netlify.com/projects/fundamental-ngx/deploys/6835fa25e13f250008518ce9
Deploy Preview https://deploy-preview-13296--fundamental-ngx.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar May 19 '25 16:05 netlify[bot]

Do not forget this https://github.com/SAP/fundamental-ngx/blob/812a1d044248da21cb6de37f87ba5eaf6bd0b524/apps/docs/project.json#L286

g-cheishvili avatar May 24 '25 20:05 g-cheishvili

  • remove the pre-commit hook as I’ve done in this PR, but we will need to understand that, when the translations are provided, we’ll need to run the script to convert the new *.properties files to *.ts.

Not only that, when translation team creates a PR, the hook and the dependsOn ensure that the .ts files are updated along the .properties. They could not launch them manually.

What I'd suggest is having additional small library or a file, which would be overriding the translation keys if there is a need for manually written texts

g-cheishvili avatar May 24 '25 20:05 g-cheishvili

closing for now and we can reopen it once it blocks us agian

droshev avatar Jul 30 '25 13:07 droshev