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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 26, 2021
Merged

Only calculate the compressed size on pull requests when necessary #32161

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2c8b38
Only calculate the compressed size when necessary.
desrosj May 24, 2021
045ba02
Merge branch 'trunk' into actions/limit-bundle-size-workflow
desrosj May 25, 2021
2e48f33
Merge branch 'trunk' into actions/limit-bundle-size-workflow
desrosj May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion .github/workflows/bundle-size.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
name: Compressed Size

on: [pull_request]
on:
pull_request:
paths:
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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.

# Any change to a CSS, Sass, or JavaScript file should run checks.
- '**.js'
- '**.css'
- '**.scss'
# Changes to any NPM related files could affect the outcome.
- '**package*.json'
# These files configures ESLint. Changes could affect the outcome.
- '**.eslint*'
# These files configures JSHint. Changes could affect the outcome.
- '**.jshint*'
# These files configures Prettier. Changes could affect the outcome.
- '**.prettier*'
# These files configures stylelint. Changes could affect the outcome.
- '**.stylelint*'
# These files configures TypeScript. Changes could affect the outcome.
- '**.tsconfig*'
# This file configures Webpack. Changes could affect the outcome.
- 'webpack.config.js'
# Changes to this workflow file should always verify the changes are successful.
- '.github/workflows/bundle-size.yml'

# Cancels all previous workflow runs for pull requests that have not completed.
concurrency:
Expand Down