-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Display Build Size Difference #340
Display Build Size Difference #340
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
@@ -77,7 +77,8 @@ | |||
"bundle-deps": "1.0.0", | |||
"react": "^15.3.0", | |||
"react-dom": "^15.3.0", | |||
"react-test-renderer": "^15.3.0" | |||
"react-test-renderer": "^15.3.0", | |||
"recursive-readdir": "^2.0.0" |
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.
This should be in dependencies of this project.
Just signed the Contributor License Agreement, moved the devDependency to a dependency, squashed, and force pushed to this feature branch |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
A few notes:
|
console.error(err.message || err); | ||
process.exit(1); | ||
function sizeDifference(currentSize, previousSize) { | ||
if (previousSize === undefined) { return ''; } |
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.
Code style: please only use multiline blocks.
if (...) {
...
}
@gaearon Ahh, I didn't realize about the previous meaning of yellow. I changed it in hopes the size difference would stand out. I like your idea about color thresholds. I wasn't sure what to do with a 2nd build when nothing changed, so I decided to show Sure, I'll update the multiline block. Thanks for the feedback. I'll work on those things tonight and update the PR. Do you want to me squash the commits or do you prefer keeping them separate so you can see the difference? Also, do you want me to rebase against master before I push my updates? |
@gaearon Btw I think white should be avoided as some people might still have white as the terminal's background? 😃 |
Sorry, I meant “default” color (no chalk). Great point 👍 |
Please rebase against master but don’t worry about squashing—GH will do that. I only ask to rebase in case I merge something conflicting in the meantime. I don’t expect to though. |
Cool, I'll keep that in mind. Thanks @gaearon @alexzherdev for the clarification
👍 |
Okay, @gaearon I made the adjustments you requested. Since I'm now only showing file-size difference for files that changed I needed to add I fetched/merged from upstream/master and rebased this feature branch based on that. If you find anything else please let me know and I'll be happy to update this PR. Thanks! |
Thank you! |
This was released in 0.2.2 but I forgot to add a changelog entry. |
facebook#339 exclude coverage from tslint.json
This PR closes #326
Test Plan
npm run build
without abuild
foldernpm run build
with abuild
folder with no changes(+0 B)
npm run build
with abuild
folder after adding new code(+3 B)
npm run build
with abuild
folder after removing old code.(-3 B)