-
Notifications
You must be signed in to change notification settings - Fork 400
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 #5810
Add bundlesize #5810
Conversation
Codecov Report
@@ Coverage Diff @@
## master mozilla/addons-frontend#5810 +/- ##
=======================================
Coverage 97.72% 97.72%
=======================================
Files 228 228
Lines 5722 5722
Branches 1101 1101
=======================================
Hits 5592 5592
Misses 115 115
Partials 15 15 Continue to review full report at Codecov.
|
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.
I like the approach -- seems simple.
Did you read this issue? siddharthkp/bundlesize#93 Looks like it's still open. I guess we need to set up a github token as a workaround.
@@ -25,16 +25,16 @@ cache: | |||
|
|||
# Allow to display job names on Travis-CI, see: | |||
# https://github.com/travis-ci/travis-ci/issues/5898#issuecomment-362490313 | |||
script: yarn $COMMAND | |||
script: FORCE_COLOR=0 yarn $COMMAND |
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.
Good idea
@@ -8,8 +8,12 @@ | |||
"node": ">=6 <=8" | |||
}, | |||
"scripts": { | |||
"build": "bin/build-checks.js && better-npm-run build", | |||
"build": "npm run clean && better-npm-run build", | |||
"build-all": "npm run clean && NODE_APP_INSTANCE=amo better-npm-run build && NODE_APP_INSTANCE=disco better-npm-run build", |
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.
But what if we add a new app??! I'm only kidding. I hope we never do that.
I did but is it still needed? Is it for the PR integration? The build will fail if it goes above the limit, so we are good I think. We need a token with a lot of permissions and I cannot setup one myself, not enough rights. |
When you merge this PR to master, it will fail because of that issue. We need to think of a workaround before merging this. I think I looked into setting up a token but a Mozilla admin I talked to was reluctant because it gives a lot of access rights to their repo. I don't recall the details and maybe it has changed since. |
I am not sure to follow because the "pr" build is green too. When there is no github token in the env (which is the case), there is no issue like the one you reported. |
It's only green because it's on a branch. They have a bug in there code that only affects builds running on the master branch.
Not having a github token is what triggers the issue. Here was my original patch that passed in a branch but failed when it landed on master: https://github.com/mozilla/addons-frontend/pull/2920/files |
@kumar303 Are you sure there was not a token set? I removed a github token in our Travis CI settings yesterday. I think your branch passed because you submitted it from a fork (no secret vars available) and did not pass on I am still investigating but I don't see where in the code the |
So I believe it works because the previous PR had a polluted travis-ci environment. |
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.
So I believe it works because the previous PR had a polluted travis-ci environment.
🤷♀️ OK, let's try it and see if it breaks.
Cool! It didn't fail on master 😅 |
Fix mozilla/addons#10656
Fix mozilla/addons#10534
I was curious to see what bundlesize could do.
This PR updates the
build
commands and removes the undocumentedADDONS_FRONTEND_BUILD_ALL
environment variable.Bundlesize checks both amo and disco apps. I also added
FORCE_COLOR=0
to remove some colors in the logs (bundlesize and chalk calls), since
logs are big and we need the raw output every time.
Output on Travis-CI