Update sizebot to new workflow #20719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
acdlite
force-pushed
the
update-sizebot-to-new-workflow
branch
6 times, most recently
from
ba4aafb
to
72b4a92
Compare
acdlite
force-pushed
the
update-sizebot-to-new-workflow
branch
8 times, most recently
from
8a3614c
to
bffa377
Compare
acdlite
changed the title
acdlite
commented
Feb 3, 2021
.circleci/config.yml
Outdated
| - run: | ||
| name: Download artifacts for base revision | ||
| # TODO: Commit sha is hardcoded until required changes land in main | ||
| # branch. Once they do, replace with `merge-base HEAD origin/master` |
There was a problem hiding this comment.
I'll un-hardcode the base sha before landing. Need to land in two stages.
acdlite
force-pushed
the
update-sizebot-to-new-workflow
branch
2 times, most recently
from
ca6bd97
to
2242ed6
Compare
bvaughn
approved these changes
Feb 3, 2021
.circleci/config.yml
Outdated
| git fetch origin master | ||
| echo "$(git merge-base HEAD origin/master)" | ||
| cd ./scripts/release && yarn && cd ../../ | ||
| scripts/release/download-experimental-build.js --commit=4783999 |
There was a problem hiding this comment.
So long as we remember to change this.
Alternative to parsing an arbitrary package's version number, or its `build-info.json`.
I think this only reason we needed this was to support passing any job id to `--build`, instead of requiring the `process_artifacts` job. And to do that you needed to use the workflows API endpoint, which requires an API token. But now that the `--commit` argument exists and automatically finds the correct job id, we can just use that.
Uses download-experimental script and places the base artifacts into a top-level folder.
acdlite
force-pushed
the
update-sizebot-to-new-workflow
branch
from
2242ed6
to
9d0ee0b
Compare
Replaces the two separate sizebot jobs (one for each channel, stable and experimental) with a single combined job that outputs size information for all bundles in a single GitHub comment. I didn't attempt to simplify the output at all, but we should. I think what I would do is remove our custom Rollup sizes plugin, and read the sizes from the filesystem instead. We would lose some information about the build configuration used to generate each artifact, but that can be inferred from the filepath. For example, the filepath `fb-www/ReactDOM-dev.classic.js` already tells us everything we need to know about the artifact. Leaving this for a follow up.
The download script will poll the CircleCI endpoint until the build job is complete; it should also poll the GitHub status endpoint if the build job hasn't been spawned yet.
|
I think something is wrong. #20746 is supposed to increase the size, but it shows a decrease. The summary (0% change) also doesn't match what the table is reporting. |
koto
pushed a commit
to koto/react
that referenced
this pull request
Jun 15, 2021
* build-combined: Fix bundle sizes path * Output COMMIT_SHA in build directory Alternative to parsing an arbitrary package's version number, or its `build-info.json`. * Remove CircleCI environment variable requirement I think this only reason we needed this was to support passing any job id to `--build`, instead of requiring the `process_artifacts` job. And to do that you needed to use the workflows API endpoint, which requires an API token. But now that the `--commit` argument exists and automatically finds the correct job id, we can just use that. * Add CI job that gets base artifacts Uses download-experimental script and places the base artifacts into a top-level folder. * Migrate sizebot to combined workflow Replaces the two separate sizebot jobs (one for each channel, stable and experimental) with a single combined job that outputs size information for all bundles in a single GitHub comment. I didn't attempt to simplify the output at all, but we should. I think what I would do is remove our custom Rollup sizes plugin, and read the sizes from the filesystem instead. We would lose some information about the build configuration used to generate each artifact, but that can be inferred from the filepath. For example, the filepath `fb-www/ReactDOM-dev.classic.js` already tells us everything we need to know about the artifact. Leaving this for a follow up. * Move GitHub status check inside retry loop The download script will poll the CircleCI endpoint until the build job is complete; it should also poll the GitHub status endpoint if the build job hasn't been spawned yet. * Only run get_base_build on main branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces the two separate sizebot jobs (one for each channel, stable and experimental) with a single combined job that outputs size information for all bundles in a single GitHub comment.
I didn't attempt to simplify the output at all, but we should. I think what I would do is remove our custom Rollup sizes plugin, and read the sizes from the filesystem instead. We would lose some information about the build configuration used to generate each artifact, but that can be inferred from the filepath. For example, the filepath
fb-www/ReactDOM-dev.classic.jsalready tells us everything we need to know about the artifact. Leaving this for a follow up.