Skip to content

Conversation

@bradleymackey
Copy link
Contributor

@bradleymackey bradleymackey commented Aug 13, 2023

  • Fixes the size compression report in (chore) add gzip size compression report #3400 by using GitHub's recommended workflow for posting comments on PR originating from forks from GitHub Actions. The steps for this workflow are outlined in this blog post.
    • In summary, pull_request does not have repo write permissions on the repository (so it can't leave a comment on incoming PRs from forks). pull_request_target does, but is unsafe to use as an event for checking-out untrusted PRs from forks. We should use pull_request followed by workflow_run to post the comment to the PR.
  • The first (pull_request) step will run npm run build-cdn on the base and the PR, then generate the markdown report which is saved as an artifact to the GitHub Actions service. This triggers the second (workflow_run) step, which reads this report and posts it as a comment (without checking-out or running untrusted code from a fork).
  • The workflow_run will only be run for PRs targeting the base branch (main).
  • Computes gzip size for all minified {js,css} files in the build directory.
  • Leaves a new comment each time the workflow is run.

Changes

  • Remove old size compression report workflow, as it couldn't comment on PRs from forks.
  • Create new size report script for comparing 2 build directories, outputting markdown.
  • Create 2 new GitHub Actions workflows for building and commenting the size report on PRs.
    • Test/demo here. Example with changes:
Collapsed Expanded
Screenshot 2023-08-14 at 09 43 23 Screenshot 2023-08-14 at 09 43 28

Checklist

  • Added markup tests this is a CI change only
  • Updated the changelog at CHANGES.md

@bradleymackey bradleymackey changed the title (chore) Fix CDN build size report (chore) Fix build size report Aug 13, 2023
@joshgoebel
Copy link
Member

Let me know when you're done here. :) Looks pretty awesome.

@bradleymackey
Copy link
Contributor Author

@joshgoebel I think it's ready now! A last possible change is whether or not we want only 1 sticky comment that gets edited each time changes are pushed, or a new comment each time (current behaviour).

@joshgoebel
Copy link
Member

I think new comment is good.

@joshgoebel joshgoebel force-pushed the chore/new-size-report branch from db1480f to 2d62846 Compare August 17, 2023 02:53
@joshgoebel joshgoebel merged commit e71d91d into highlightjs:main Aug 17, 2023
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

Successfully merging this pull request may close these issues.

2 participants