-
Notifications
You must be signed in to change notification settings - Fork 894
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
Cache dependencies and archive builds in all workflows #8631
base: main
Are you sure you want to change the base?
Conversation
|
Size Report 1Affected ProductsNo changes between base commit (e3e2078) and merge commit (848646f).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (e3e2078) and merge commit (848646f).Test Logs |
a9f9c96
to
a50ccc7
Compare
node-version-file: '.nvmrc' | ||
cache: yarn | ||
cache-dependency-path: yarn.lock | ||
- name: Restore cached node_modules |
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.
Do you have any metrics as to how much faster the act of caching is?
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.
So far what I've gathered is that without caching yarn install
takes ~60s in CI, and the resulting node_modules
is ~935MB. Restoring a cached node_modules
(after a cache hit) takes only ~7s, and then the subsequent yarn install
takes 5s.
I'll include more details in a section about speed changes in my PR description in a bit
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.
Noice.
.github/workflows/test-all.yml
Outdated
path: "**/node_modules" | ||
key: node_modules-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('yarn.lock') }} | ||
- run: yarn install --frozen-lockfile | ||
- name: install Chrome stable |
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 previous workflow started with the chrome installation as it seems to affect some tools installation. We're sure that it's safe to push it down this far?
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.
This is an oversight by me- I shouldn't have moved it down. It looks like it's passing though? https://github.com/firebase/firebase-js-sdk/actions/runs/11706621527/job/32604805660
Do you think I should move it back anyway? I'm not familiar with how this works
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 don't remember what it affected but I think it might be the chromedriver yarn install in the auth package. There might possibly be a hiccup on that install if it can't find some Chrome file. Or maybe not, maybe it doesn't matter if Chrome is there until the karma test begins to run. May be a comment somewhere in the old PRs messing with the Chrome installation in this workflow and chromedriver versions.
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 think it's worth moving it back to the top so that it stands out more.
.github/workflows/release-tweet.yml
Outdated
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 forgot we still had this workflow, by the way, I think we can get rid of it.
draft