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

Have binary size check for amp4ads-v0.js #23307

Closed
lannka opened this issue Jul 12, 2019 · 9 comments
Closed

Have binary size check for amp4ads-v0.js #23307

lannka opened this issue Jul 12, 2019 · 9 comments

Comments

@lannka
Copy link
Contributor

lannka commented Jul 12, 2019

As to ensure the success of the inabox-lite project #22867, we would like to monitor the binary size for amp4ads-v0.js in the same way as v0.js.

@rsimha is this something your team can help? Thanks

@rsimha
Copy link
Contributor

rsimha commented Jul 12, 2019

Assigning to @danielrozenberg.

@lannka I assume you're interested in the brotli compressed size? (See related issue #21275)

@lannka
Copy link
Contributor Author

lannka commented Jul 13, 2019

@rsimha yep, that sounds better. Thanks!

@lannka
Copy link
Contributor Author

lannka commented Jul 23, 2019

Can we prioritize this item? We just got a bundle size regression: #23487

It's very easy to pull in regular AMP code to AMP4ADS and cause a big size regression.

@rsimha @danielrozenberg

@lannka
Copy link
Contributor Author

lannka commented Jul 26, 2019

@zhouyx do you want a size check for amp-analytics-0.1.js ? :-)

@rsimha
Copy link
Contributor

rsimha commented Jul 26, 2019

Adding @erwinmombay who is going to work on some of the bundle-size feature requests.

@zhouyx
Copy link
Contributor

zhouyx commented Jul 26, 2019

I'm trying to think what's the value for such a check for amp-analytics.js. I could definitely see the value if later on we lazy load feature bundles, but not right now.

It would be great to introduce such check for amp-analytics.js, but I also don't think that's super urgent right now. Really based on the effort and the cost on adding such check.

@lannka
Copy link
Contributor Author

lannka commented Aug 26, 2019

/cc @powerivq

@rsimha
Copy link
Contributor

rsimha commented Aug 26, 2019

Un-assigning @erwinmombay who no longer works on this. Tracking the bundle size of other binaries is definitely possible, although it will need non-trivial changes on the amp-github-apps side.

/cc @ampproject/wg-infra

@danielrozenberg
Copy link
Member

Fixed

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

No branches or pull requests

5 participants