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

feat(ci): add size-limit and apply it to openapi-fetch #1859

Merged
merged 1 commit into from
Aug 17, 2024

Conversation

yoshi2no
Copy link
Contributor

Changes

ref: #1853

  • added a workflow named "Size Limit" using size-limit-action to check the size of openapi-fetch on the main branch.
  • added a size-limit section to the root package.json and defined the limit.
    • set brotli to false because I believe it’s better to define the limit based on simply minified values.
    • I’ve set the limit to 6.5 kB for now, but I would appreciate your input on what the ideal size should be.
  • (optional) added an npm script called size to allow checking the size locally. This is optional, so feel free to remove it if it’s not needed.
  Size limit: 6.5 kB
  Size:       5.96 kB with all dependencies and minified

How to Review

  • Ensure that the GitHub Actions bot comments on the PR with the results of size-limit.

Checklist

N/A

@yoshi2no yoshi2no requested a review from a team as a code owner August 17, 2024 08:00
Copy link

changeset-bot bot commented Aug 17, 2024

⚠️ No Changeset found

Latest commit: 44fe4ba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@yoshi2no
Copy link
Contributor Author

yoshi2no commented Aug 17, 2024

Currently, there are no size-limit-related dependencies in the main branch, which seems to be causing an error 👀

  • size-limit
  • @size-limit/preset-small-lib

@yoshi2no yoshi2no changed the title feat(ci): add size-limit feat(ci): add size-limit and apply it to openapi-fetch Aug 17, 2024
"size-limit": [
{
"path": "packages/openapi-fetch/dist/index.min.js",
"limit": "6.5 kB",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be replaced with a more appropriate value.

Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you so much for adding!

Yes I understand these checks do get weird in PRs. But since that check isn’t blocking (currently) it’s fine for now. Thanks again!

@drwpow drwpow merged commit adb9372 into openapi-ts:main Aug 17, 2024
7 of 8 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.

2 participants