-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
misc(build): report to buildtracker on commit via ghactions #10550
Conversation
.github/workflows/buildtracker.yml
Outdated
- run: yarn --frozen-lockfile | ||
- run: yarn build-all | ||
|
||
- name: Store in buildtracker |
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.
wdyt of just adding this to the basic workflow? You need to install node, run yarn and build for this to work, so it's already a heavy workflow.
#10549 reused basic.yml for the same reason.
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.
good call.
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.
You need to install node, run yarn and build for this to work, so it's already a heavy workflow
why is installing node 10 so heavy? I don't understand how e.g. in https://github.com/GoogleChrome/lighthouse/pull/10549/checks?check_run_id=565300864 protoc, python, and pip run in like 3% of the time node setup does. Obviously protoc is pre-compiled, but what's the lazy node action doing in there?
If it stays at 2.5 minutes I foresee all actions going in this file :)
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.
why is installing node 10 so heavy?
dont know.. i see it taking a while today but in previous master commits, that step takes <3s.
.github/workflows/buildtracker.yml
Outdated
run: yarn bt-cli upload-build | ||
env: | ||
# https://buildtracker.dev/docs/guides/github-actions#configuration | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
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 GITHUB_TOKEN
is already a set env variable.
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.
good call.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
update: all fixed. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
package.json
Outdated
@@ -74,6 +74,7 @@ | |||
"static-server": "node lighthouse-cli/test/fixtures/static-server.js" | |||
}, | |||
"devDependencies": { | |||
"@build-tracker/cli": "^1.0.0-beta.14", |
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.
theres beta 15 now paularmstrong/build-tracker@v1.0.0-beta.14...v1.0.0-beta.15
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.
good catch. i actually asked him to publish that one. :)
it fixed the slow responsiveness of the hoverline on https://lh-build-tracker.herokuapp.com/
Really awesome to see this coming together!
If there's anything that you think could make things easier, would love to see feature suggestions! |
I've set up https://buildtracker.dev/ and backfilled it with the last 18months of commits:
https://lh-build-tracker.herokuapp.com/builds/limit/1000
The UX takes some clicking around to get used to but it's pretty powerful. I like this better than the alternatives mostly for the rich UI of exploring historical commits.
This PR adds a github action workflow that will report to buildtracker on every PR push and master commit. (I considered using the buildtracker action, but it breaks on
push
and currently always posts a PR comment). By default buildtracker only plots master branch but all the data is there in case we wanted to plot other branches.Here's an interesting find from our recent commits... 21K increase in dt-bundle. It's attributable to the
update axe-core to 3.5.1
commit, natch. 2-commit comparison linkFollowups:
ref #10472