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

Introduce macros in the translation files #802

Merged
merged 3 commits into from
Oct 24, 2021
Merged

Introduce macros in the translation files #802

merged 3 commits into from
Oct 24, 2021

Conversation

baltpeter
Copy link
Member

No description provided.

@baltpeter baltpeter added cleanup i18n Internationalization labels Oct 24, 2021
@baltpeter baltpeter requested review from zner0L and mal-tee October 24, 2021 11:33
@cypress
Copy link

cypress bot commented Oct 24, 2021



Test summary

65 0 7 0Flakiness 0


Run details

Project datenanfragen/website
Status Passed
Commit 3e04476
Started Oct 24, 2021 12:51 PM
Ended Oct 24, 2021 12:53 PM
Duration 02:52 💡
OS Linux Debian - 10.3
Browser Electron 91

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Member

@mal-tee mal-tee left a comment

Choose a reason for hiding this comment

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

Thank you!

Could you add a little explanation to the i18n README as well? And maybe notify our language maintainers about this change (after we merged this). :D

Seems like something broke in the footer: The nbsp isn't rendered.
grafik

scripts/webpack-i18n-loader.js Outdated Show resolved Hide resolved
scripts/webpack-i18n-loader.js Show resolved Hide resolved
@mal-tee
Copy link
Member

mal-tee commented Oct 24, 2021

We should also write a test that checks if a macro is defined, if it's used in the translations. Feel free to move this into a new issue.

@baltpeter
Copy link
Member Author

And maybe notify our language maintainers about this change (after we merged this). :D

Why? I don't see a need for that. Isn't this equivalent to using a variable instead (which we are already doing in lots of places)?

@mal-tee
Copy link
Member

mal-tee commented Oct 24, 2021

And maybe notify our language maintainers about this change (after we merged this). :D

Why? I don't see a need for that. Isn't this equivalent to using a variable instead (which we are already doing in lots of places)?

Just as a heads-up that things changed. I thought they might want to use that feature as well, e.g. for language-specific things.

But we don't have to do that.

Copy link
Member

@mal-tee mal-tee left a comment

Choose a reason for hiding this comment

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

Thank you!

@zner0L
Copy link
Member

zner0L commented Oct 27, 2021

Huh, I want to remove dependency on this script, not introduce more… I have to fix all of this in #701

@baltpeter
Copy link
Member Author

I know, but we need some processing on the translations files anyway. And this is a huge win for maintaining the different language versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup i18n Internationalization
Development

Successfully merging this pull request may close these issues.

3 participants