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

Asset Size Check Improvements #6677

Open
2 of 10 tasks
runspired opened this issue Nov 5, 2019 · 2 comments
Open
2 of 10 tasks

Asset Size Check Improvements #6677

runspired opened this issue Nov 5, 2019 · 2 comments
Labels
6.0 Roadmap good-first-issue Infra/CI/DX 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166

Comments

@runspired
Copy link
Contributor

runspired commented Nov 5, 2019

The AssetSizeCheck that runs on PRs could be improved in the following ways, listed below in the order of priority

  • We should update the comment in place instead of deleting it and posting a new one
  • bot to post and manage the results comment since PRs from forks don't have permissions
  • ability for data core team members to approve an asset size increase via a comment ala @github-action AssetSizeIncreaseOK <some description of why> which will cause CI to pass for the increase while still reporting it.
  • either manually specify a list or diff the output of ember new to get a list of all files added to app/ by ember-data so that we can correctly track them as part of our footprint
  • add parallel tracking of our size when not supporting IE11
  • upload to S3 or similar a per-commit-to-master history of our asset size for tracking
  • app to view asset size history over time

Minimum Data Check

  • a "minimum possible ember-data" app (similar to the encapsulation test apps) that lets us track the smallest ember-data implementation with the rough basics of management for network, cache, presentation, and mutation

Because we conditionally remove code that exists to support specific legacy packages (notably model/adapter/serializer) the reported size of store which represents the essential core of ember-data is over-reported. As we work to rationalize the network layer this over-reporting will become even more substantial. It is already likely close to 30% of the reported size of store, and as we continue to deprecate the legacy model world it's likely this number grows as high as 70%.

We should build a tiny package that just installs the store package (the only essential package at this point) and reports it's size. Since most folks will likely choose to use our cache as well we should probably build a similar scenario for those two packages.

Improved Accounting

  • the asset-size check occasionally displays wrong math
  • adjust the mechanism of the check to account for v2-addon builds

An example of this is in #8078 where a reduction of about 3.5kb compressed is correct in the total but displays as 75kb in the relative change.

Related #8086 #8103

@runspired runspired added Good for New Contributors 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166 labels Nov 5, 2019
@runspired
Copy link
Contributor Author

Related: #6678

@runspired
Copy link
Contributor Author

Now that our existing infra has had time to mature we've come up with a few tweaks to improving it:

1 - we need a bot to post and manage the results comment since PRs from forks don't have permissions
2- We should update the comment in place instead of deleting it and posting a new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.0 Roadmap good-first-issue Infra/CI/DX 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166
Projects
Status: No status
Development

No branches or pull requests

1 participant