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

🏗 build and copy story localization strings to dist on amp build and amp dist #37623

Merged
merged 12 commits into from Feb 9, 2022

Conversation

erwinmombay
Copy link
Member

We need to start deploying these json files on the Google AMP cache as we will soon lazy load amp-story strings to reduce the size of the story JavaScript binaries.

…`amp dist`

We need to start deploying these json files on the Google AMP cache as
we will soon lazy load amp-story strings to reduce the size of the story
JavaScript binaries.
@mszylkowski
Copy link
Contributor

Maybe it's worth clearing this out: is it worth "merging the fallbacks" in this step or in the cache + local server? I have a feeling that it might be easier to do here since it's just one place, but you probably know better.

@erwinmombay
Copy link
Member Author

@mszylkowski yes, probably here. let me push an update

Copy link
Member

@danielrozenberg danielrozenberg left a comment

Choose a reason for hiding this comment

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

Approval for build-system style and syntax, I assume someone else reviewed the generator code for correctness

build-system/tasks/build-story-localization.js Outdated Show resolved Hide resolved
build-system/tasks/build-story-localization.js Outdated Show resolved Hide resolved
build-system/tasks/build-story-localization.js Outdated Show resolved Hide resolved
erwinmombay and others added 3 commits February 9, 2022 11:07
Co-authored-by: Daniel Rozenberg <me@danielrozenberg.com>
Co-authored-by: Daniel Rozenberg <me@danielrozenberg.com>
@erwinmombay
Copy link
Member Author

@danielrozenberg could you take another look. added --watch functionality

Copy link
Contributor

@mszylkowski mszylkowski left a comment

Choose a reason for hiding this comment

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

👍 after Justin's comments

build-system/tasks/build-story-localization.js Outdated Show resolved Hide resolved
@danielrozenberg
Copy link
Member

@danielrozenberg could you take another look. added --watch functionality

👍

@amp-owners-bot
Copy link

amp-owners-bot bot commented Feb 9, 2022

Hey @jridgewell! These files were changed:

build-system/babel-plugins/babel-plugin-transform-json-import/index.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants