-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Only calculate the compressed size on pull requests when necessary #32161
Conversation
This updates the compressed size workflow to run only when the files changes would have an impact on the package size.
Size Change: 0 B Total Size: 1.86 MB ℹ️ View Unchanged
|
on: [pull_request] | ||
on: | ||
pull_request: | ||
paths: |
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.
The problem with this config is that it prevent us from making the job "Required" on GitHub because it will make the PRs that don't need the check to get stuck forever.
The potential solution for this would be to use if:
inside the job itself instead of paths
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.
I'm proposing that this workflow is no longer Required for PRs on GitHub.
Even if the bundle size increases exponentially, the workflow will still pass. The merging contributor must review the comparison posted in a comment on the PR and decide if the difference is acceptable before approving the PR.
Since the contributor must review the comment manually anyway, I think this is not that big of a change.
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.
Yes, I think it's fine for this workflow.
This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3)
This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3)
This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3)
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <kevin830726@gmail.com> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <adam@adamziel.com> Co-authored-by: Kerry Liu <gwwar@users.noreply.github.com>
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <kevin830726@gmail.com> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <adam@adamziel.com> Co-authored-by: Kerry Liu <gwwar@users.noreply.github.com>
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <kevin830726@gmail.com> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <adam@adamziel.com> Co-authored-by: Kerry Liu <gwwar@users.noreply.github.com>
Description
This updates the compressed size workflow to run only when the files changes would have an impact on the package size.
Currently, this workflow is a requirement for merging a pull request. However, even if the bundle size increases exponentially, the workflow will still pass. The merging contributor must review the comparison posted in a comment on the PR and decide if the difference is acceptable before approving the PR.
Since the contributor must review the comment manually anyway, I propose that the requirement for this workflow is removed so that path scoping can be used to limit how often this workflow runs, preventing unnecessary runs when the files changed will not impact the plugin build.
Checklist:
*.native.js
files for terms that need renaming or removal).