bundlesize
bundlesize copied to clipboard
siddharthkp
Change to master isn't shown in pull request status
Paraphrased by @siddharthkp
Being able to see the change is very motivating, especially when you're trying to reduce it.
Currently, I can only see the change when clicking on the 'Details' link (The status message is not very helpful)
What can we improve to make the default message better/communicate changes upfront?
Original issue description is folder here:
Do you want to request a feature or report a bug? A bug, although it may be a feature request if this is intentional.
What is the current behavior?
Currently, my pull request tells me that I've done a good job and that my bundles are within limits.
If the current behavior is a bug, please provide the steps to reproduce. I have no idea, but here is my configuration:
"bundlesize": [
{
"path": "./bundlesize/app.js",
"maxSize": "75 kB"
},
{
"path": "./bundlesize/vendor.js",
"maxSize": "350 kB"
},
{
"path": "./bundlesize/app.css",
"maxSize": "50 kB"
},
{
"path": "./bundlesize/vendor.css",
"maxSize": "10 kB"
}
]
What is the expected behavior?
My pull request doesn't just tell me I did a good job, but also tells me the change in size.
If this is a feature request, what is motivation or use case for changing the behavior? Being able to see the change is very motivating, especially when you're trying to reduce it. Currently, I can only see the change when clicking on the 'Details' link.
Please mention other relevant information.
- node version 8.9.x
- npm version not used, yarn 1.3.2
- Operating system ubuntu
- bundlesize version 0.15.3
- CI you are using Codeship
Being able to see the change is very motivating, especially when you're trying to reduce it. Currently, I can only see the change when clicking on the 'Details' link.
Yep , that's a thing I'm not super happy about. Do you have suggestions on how we can do better in the case of multiple files?
(P.S. I'm going to make the issue description a bit tinier to get the feature request across, I hope you don't mind)
We're experimenting with this on a fork, but it requires additional permissions (commenting on PRs). There's just not enough room on build status.
@threehams
There's just not enough room on build status
that's what I struggled with 😔
@siddharthkp no problem, original point comes across very well with your edits!
@threehams In our case we're checking the size of four files. Having all of those featured in the status is indeed not feasible due to space restrictions.
If comments require additional permissions, that'd probably be suboptimal indeed. But, what about the next two suggestions?
One status for each file:
Taking inspirtation from Codecov which recently introduced "flags". These flags can be used to divide the codebase into different sections where different rules (and thus a different status) can be applied. It looks good and can contain all the information we need:
Combine statistics of all checked files in the message:
Probably the simplest change, but a little bit less cool. This would still give you an insight in the amount of change you have done, although you can't see it for each individual file without clicking the Details link.
I'd definitely be willing to help out with either of these. Although the first has my preference, the second is already a very good improvement.
There's one downside with the second version, though. For example, if we have a file A with a limit of 10 and B with a limit of 100. That's 110 total. But what if the actual size of A is 20 (twice the limit) while B is 50? That'd be displayed as 70/110, yet it would fail. That may cause some confusion.
I really like idea number 2! the first one is more explicit but also more noisy 😢
One tiny optimisation that can make no. 1 better would be to skip the files that have no change.
Personally, I think if there's only one bundle that's effected, we should show that bundle and leave the rest to the details page. If it's more than 1, switch to idea no. 2
Skipping files with no change would not allow repositories to mark a status check as required, which is a bit of a bummer as that way you can't force your co-workers to keep it within limits 👼
Personally, I think if there's only one bundle that's effected, we should show that bundle and leave the rest to the details page. If it's more than 1, switch to idea no. 2
Yeah, I agree. Perhaps this could be the behavior for different situations:
- one file, passes checks: "app.js is 46.88/75kB (+1B)"
- one file, fails checks: "app.js is too big! 76.88/75kB (+2B)"
- multiple files, all pass checks: "Total bundle size is 360.07/485kB (+8B)"
- multiple files, one fails checks: "app.js is too big! 76.88/75kB (+2B)"
- multiple files, multiple fail checks: "Some of your files became too big! (+2B)"
Perhaps the messages can be a bit cleaner, but I think it'd work.
Edit: Looking at the code I can have this done by tomorrow. Since this change would also benefit the company I work for, they won't have any issue with me working on this.
I like the idea of multiple statuses, but it would definitely need to be configurable. We're a bit edge case in that we'll (eventually) have over 1000 individual files to size-check, so that'd be a bit much.
@threehams I'll take multiple statuses into account, so that when the time is right it should be easy to implement.
For now though, I'll focus on getting better messages in one status for multiple files. I've tested my initial changes against our project and the messages work correctly, but somehow (even though I can see the change compared to master in the status message) the details page doesn't show the values for master anymore. But hey, progress is being made!
@siddharthkp Could you take a look at #183, please? The messages are working correctly, but there's two issues that I don't understand. I've listed these in the description of the pull request. I'm hoping someone with (much) more experience in this project (you) will be able to see where I messed up.
In my use case, the total value is not good enough.
Monitoring some specific files is really important, like entry script main.js, a small change but create a big change in its size (however, it's not higher the maxSize)
