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

Add bundlesize status check #1209

Closed
1 task done
dr3 opened this issue Jan 29, 2019 · 9 comments
Closed
1 task done

Add bundlesize status check #1209

dr3 opened this issue Jan 29, 2019 · 9 comments
Assignees
Milestone

Comments

@dr3
Copy link
Contributor

dr3 commented Jan 29, 2019

Is your feature request related to a problem? Please describe.
As we develop while on live we should be very focused on not bloating our application by mistake.

Describe the solution you'd like
Add a bundlesize checker script such as https://github.com/siddharthkp/bundlesize and pass it a github token via Jenkins credentials.

OR we could add a BUNDLESIZE_GITHUB_TOKEN var to travis, as it has a simple integration

Describe alternatives you've considered
Writing our own script 👎

@dr3 dr3 added Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. should labels Jan 29, 2019
@ChrisBAshton ChrisBAshton added v1.1 and removed v1.0 labels Mar 13, 2019
@rossgaskell
Copy link
Contributor

Resolved in #1586

@dr3 dr3 reopened this Jun 20, 2019
@dr3
Copy link
Contributor Author

dr3 commented Jun 20, 2019

We currently don't have any way of monitoring bundle size per PR, so this isnt resolved. ATM we just creep up to the limit, and are forced to increase it.

@chris-hinds
Copy link
Contributor

Will look into this, I was going to see if there was a way travis could comment on our PR with the bundle size, or add it as a status anyway. Would be so much easier to understand what PR affected the bundles

@dr3
Copy link
Contributor Author

dr3 commented Jun 20, 2019

@hindsc52 Cheers, bundlesize has a easy travis integration which should cover all our bases I think :)

@jamesbhobbs jamesbhobbs added this to the Simorgh 3.0 milestone Aug 20, 2019
@chris-hinds chris-hinds removed the Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. label Nov 24, 2019
@chris-hinds
Copy link
Contributor

As we no longer "require" travis this integration should live in our Jenkins pipeline.

@samora samora assigned samora and unassigned samora Nov 26, 2019
@andrewscfc andrewscfc self-assigned this Nov 27, 2019
@andrewscfc
Copy link
Contributor

Following discussion with @hindsc52 we've decided to adapt our own bundle size analysis implementation and report it's result as a status on github: https://developer.github.com/v3/repos/statuses/#create-a-status

We have found that the bundlesize plugin maintainers have chosen to not support checking of the total bundle size (siddharthkp/bundlesize#205 (comment)) but instead favour ongoing monitoring of individual chunk file sizes. We believe this would result in too many status checks in our case.

As an open question how many status checks would we want?

  • One for the main bundle, the vendor bundle and for each service?
  • One for the main bundle and the vendor bundle?
  • Something else?

Typical bundles output for context:

afaanoromoo-31ecd969.c2f5879f.js
afrique-31ecd969.3f29dd14.js
amharic-31ecd969.daa6f3cf.js
arabic-31ecd969.e58ea2f5.js
azeri-31ecd969.22c19e60.js
bengali-31ecd969.bf84dbc9.js
burmese-31ecd969.41177b72.js
cymrufyw-31ecd969.9344ee45.js
gahuza-31ecd969.2b400723.js
gujarati-31ecd969.a1a8c3a5.js
hausa-31ecd969.15bb3333.js
hindi-31ecd969.2332e07a.js
igbo-31ecd969.6227b678.js
indonesia-31ecd969.9e6113d3.js
japanese-31ecd969.4150af70.js
korean-31ecd969.98ae788a.js
kyrgyz-31ecd969.c9b52f8e.js
main-d0ae3f07.250d627c.js
marathi-31ecd969.24cd92d2.js
mundo-31ecd969.96e3cec7.js
naidheachdan-31ecd969.f63a0142.js
nepali-31ecd969.d9a1bee7.js
news-31ecd969.2415376a.js
pashto-31ecd969.c0f6ce85.js
persian-31ecd969.cf5e8c59.js
pidgin-31ecd969.193f1326.js
portuguese-31ecd969.6fedb87f.js
punjabi-31ecd969.a3e25da0.js
rich-text-transforms-748942c6.b91d473a.js
russian-31ecd969.64a838d0.js
scotland-31ecd969.e1ff61ae.js
serbian-31ecd969.2a07ad07.js
sinhala-31ecd969.c2ada73b.js
somali-31ecd969.7e81aa7e.js
swahili-31ecd969.e144a36b.js
tamil-31ecd969.287323f3.js
telugu-31ecd969.491cd8a1.js
thai-31ecd969.b3808aa2.js
tigrinya-31ecd969.77c596a0.js
turkce-31ecd969.83aa6a69.js
ukchina-31ecd969.57775889.js
ukrainian-31ecd969.afa83aa1.js
urdu-31ecd969.2ddf3005.js
uzbek-31ecd969.8d4072f9.js
vendor-1f20a385.051100bf.js
vendor-253ae210.13360932.js
vendor-678f84af.9270382c.js
vendor-fd731fb0.343a893e.js
vietnamese-31ecd969.ef9a917e.js
yoruba-31ecd969.a19e8389.js
zhongwen-31ecd969.af964d20.js

@chris-hinds
Copy link
Contributor

hmmm we do not want to report on the service bundles just yet as that would be a lot of data in the status check (40+ services)

Maybe for now we send the main, vendor and max service bundle (i.e. just report of the largest service bundle)?

@andrewscfc
Copy link
Contributor

andrewscfc commented Nov 28, 2019

Maybe for now we send the main, vendor and max service bundle (i.e. just report of the largest service bundle)?

Sounds good to me, I'll investigate doing this and see how it goes.

@andrewscfc
Copy link
Contributor

Following discussion with @hindsc52 , we decided a status check that does not track changes over time is not helpful; the build failing for an increase already gives us what we need. Closing as agreed.

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

No branches or pull requests

8 participants