Add CI workflow to compute diff between files dist files

[Kocal]

Q	A
Bug fix?	no
New feature?	no
Issues	Fix #...
License	MIT

This PR is purely internal, and aims to display the assets/dist/*.{js,css} files diff between 2.x and a pull-request. Similar to https://github.com/marketplace/actions/pkg-size-action, but fully internal.

I wanted a tool that display dist files size diff for each pull-request, because I was a bit afraid of changes done in https://github.com/symfony/ux/pull/2160.

When a PR is opened, it check dist files between the base branch (2.x) and the pull request, and it create a GitHub comment. The comment is created by https://github.com/marocchino/sticky-pull-request-comment, and is automatically updated depending of the check state. If any diff between dist files, then a table is displayed, with a line per file. It shows the original size and compressed (gzip and brotli) sizes, and also a difference in %.

States

Currently not working on this repository, but you can see them on https://github.com/Kocal/symfony-ux/pull/1

When opening a PR

When an issue happened

When there is no difference between base and PR

When there is difference between base and PR

[Kocal]

It looks like there are permissions issues, even after added permissions.pull-requests: write like mentioned in the documentation:

Even if I use permissions: 'write-all' it does not work either:

I suspect a configuration at organization-level...?

@kbond do you think you can do something about that please? 🙏🏻

/cc @nicolas-grekas or maybe you Nicolas since you setup the same action on Symfony recipes repositories :)

Thanks!

[kbond]

Shouldn't this job only run if a dist file has been changed?

[Kocal]

Shouldn't this job only run if a dist file has been changed?

Fixed by using the following config:

on:
  pull_request:
    paths:
      - 'src/*/assets/dist/**'
      - 'src/*/src/Bridge/*/assets/dist/**'

I will push soon, thanks

[Kocal]

I've changed the table rendering after @javiereguiluz's comment, it now looks like this:

We still need to find out why there is this Resource not accessible by integration error, and we are good IMO

[javiereguiluz]

@Kocal I like a lot what you are doing here. Thanks.

Some additional comments ... but I could be wrong, so feel free to ignore them:

• I'd remove Brotli because it's just "the same as Gzip but a bit better", so it doesn't add any relevant information
• I'd round percentages and remove decimals to make them easier to understand: -4% is better to me than -3.9% and +74% is better than +74.3%
• I'd put the percentages first, because it's the most relevant info by far. E.g. I don't care that the new file size is 5.56 KB, I just want to quickly check that it's a -27% file size change.

Thanks!

[Kocal]

Thanks Javier, the more we speak about Brotli and the more I think we can remove it (btw pkg-size-action can display Brotli sizes). Gzip is the most common compression standard for the moment, while Brotli is — I believe — still a "niche" thing. Brotli users know that their files will be smaller than gzipped files.

I agree for percentage rounding.

When it comes to putting percentages first, I don't really agree with you. Yes, percentage differences are important, but when the browser goes to download a resource, it's the size of the resource that matters, not the percentage difference. If a file is reduced from 1 MB to 500 KB, we gain 50%, but more importantly we save 500 KB (and that's a really huge gain). If a file is reduced from 10 Kb to 5 Kb, you also gain 50%, but you only save 5KB (which still has an impact, but a much smaller one). But then, your point of view is valid too and I can understand it, I think here it's two different ways of thinking :)

[Kocal]

Thanks for your reviews, the diff table now looks like this:

[javiereguiluz]

Looks very nice!! Thanks a lot Hugo.

[kbond]

This looks great!

@Kocal:

1. Can you revert the temporary commit?
2. The proper permissions are still todo?

[Kocal]

@kbond yes there are still permission issues, and I can revert the temporary commit (when permissions will be fixed, otherwise we won't see the comment)

[Kocal]

I've reworked the workflow in two dedicated workflows, hoping for permissions issues to be gone. We won't have comment before the diff is generated, or if a failure happened.

I reverted the commit that modify dist files for testing purposes. I'm merging ASAP