Skip to content

chore: Convert lang/en.lyaml to native JS objects #1418

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

Merged
merged 2 commits into from
Apr 3, 2025

Conversation

joe-yeager
Copy link
Contributor

@joe-yeager joe-yeager commented Apr 2, 2025

Description and Context

Does not delete the en.lyaml file or modify the i18n functionality at all. This PR is purely additive so we can begin the conversion.

@joe-yeager joe-yeager changed the title Jy/convert lang file to js chore: Convert lang/en.lyaml to native JS objects Apr 2, 2025
@joe-yeager joe-yeager marked this pull request as ready for review April 2, 2025 15:37
@camden11
Copy link
Contributor

camden11 commented Apr 3, 2025

Is there any advantage to adding this without making the changes to actually pull data from it? Won't it just get out of date when we make copy updates?

@joe-yeager
Copy link
Contributor Author

Is there any advantage to adding this without making the changes to actually pull data from it? Won't it just get out of date when we make copy updates?

@camden11 - good question, I was thinking when we made copy changes we could update the new file, delete the lines from the old file, and switch to use the new strings. I'm going to work on getting all of the i18n calls ported over when I get some more time

@camden11
Copy link
Contributor

camden11 commented Apr 3, 2025

That makes sense to me. I guess I don't feel super strongly, but I could see it making sense to include a version of the i18n function that works with the JSON file with this PR so we can get started with that right away

@joe-yeager
Copy link
Contributor Author

That makes sense to me. I guess I don't feel super strongly, but I could see it making sense to include a version of the i18n function that works with the JSON file with this PR so we can get started with that right away

@camden11 In the new version we wouldn't need the i18n function. This is a js file and the functions can be directly invoked

@camden11
Copy link
Contributor

camden11 commented Apr 3, 2025

Ohhhhh 🤦 . I had it in my head that this was JSON like LDL even though you had native JS in the title and everything haha.

@joe-yeager joe-yeager merged commit 5f687ea into main Apr 3, 2025
1 check passed
@joe-yeager joe-yeager deleted the jy/convert-lang-file-to-js branch April 3, 2025 23:08
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.

2 participants