-
Notifications
You must be signed in to change notification settings - Fork 524
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
fix(build): copy images that are only in default locale to l10n folder #7917
Conversation
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
@caugner, may I get a review :) |
This pull request has merge conflicts that must be resolved before it can be merged. |
1 similar comment
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
@LeoMcA I would suggest that we handle this instead in our Cloud Function here: If we don't find an asset in |
We may also need to consider mdn/translated-content#7648 |
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 and tested locally by comparing full builds of this branch vs main, just two nits (one about naming the map variable, and another about reusing findByURLWithFallback
). 🚀
Co-authored-by: Claas Augner <[email protected]>
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 a lot! 🚀
Thank you @yin1999. I triggered an xyz-build, and once that has finished, you should be able to see the results here. If all goes well, the images that are missing here on prod, should appear here on xyz. |
I'll keep an eye on the build results. Thanks for your advice as well :) |
(The live samples are broken on xyz probably because we didn't update the workflow when the playground landed.) |
Summary
Fixes: #5652
Problem
The images used in live sample (
{{EmbedLiveSample}}
) are not served (404) when they are not duplicated on mdn/translated-content.Solution
See: #5652 (comment)
Copy images to l10n built folders.
We have serve the image files correctly in
yarn-server
, so we should only copy the images when in static build mode.Screenshots
See testing.
How did you test this change?
run
yarn test:testing
.