-
Notifications
You must be signed in to change notification settings - Fork 63
Avoid error using only local translation files #367
Conversation
I'm pretty sure we can add |
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! Not sure why CI is failing though.
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. +1 to what Dhruv said. Super great to not track generated files 🙂
The |
If that file is a pre-requisite for running the tests, even locally, then we should probably add it to the test scripts. I wonder if there's a way to just add it to the jest setup though instead of having to duplicate it across all the NPM scripts. Which brings up the other option, of using But that's controversial... idk many frontend projects (if any) that don't use |
Thinking again, it makes more sense to add the get-translation step in the Github action! And keep the Edit: You beat me for a minute. |
I remembered why I had chosen to use just the |
7be1500
to
c1bb0a6
Compare
💯 |
@krysal I would add a |
Done! Thanks for your suggestions pals 😄 |
* main: Replace PDM logo (#411) Add playwright e2e tests (#394) Update breakpoints to the new Tailwind config (#408) Create a global audio player (amongst many smaller improvements) (#399) Create Skeleton components (#392) Tailwind breakpoints (#403) Avoid error using only local translation files (#367) Add base Button component (#372) Use Tailwind RTL styles everywhere (#355) Add a dependencies section and remove the redundant title (#379) 🔄 Synced file(s) with WordPress/openverse (#384) Fix filters not being set on SSR (#386)
Fixes
Fixes #298 by @sarayourfriend
Description
This addresses the problem of the project failing to run if not all translation files are present, avoiding doing
i18n:get-translations
previously.It adds a step to the
dev
command to generate the locales list. The downside of this approach is that it modifies thevalid-locales.json
file and we will need to be alert in PR changes, but do we really need to track it? Looks like it is actually not used.Testing Instructions
en.json
npm run dev
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin