Skip to content
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

(chore) Fix build size report #3840

Merged
merged 19 commits into from Aug 17, 2023

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 merged commit e71d91d into highlightjs:main Aug 17, 2023
15 checks passed
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.

None yet

2 participants