-
Notifications
You must be signed in to change notification settings - Fork 653
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
add nightly benchmark #793
Conversation
- uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0 | ||
with: | ||
go-version: ${{ steps.goversion.outputs.goversion }} | ||
- name: Run Benchmarks |
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.
maybe worth sharing the benchmark as a workflow template?
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.
Thanks, @tjungblu! I wanted to deliver this, but I've been busy with other tasks. I would suggest (following your own suggestion ;) adding it to a template to keep it DRY.
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.
cool, I'll update it once I'm back. Feel free to take it if you get there any earlier.
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'm also out, and will be back on Monday 🙈. But I also think there's no rush to deliver this.
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.
just refactored it, if you have a brief couple minutes to review - would be much appreciated @ivanvc
02ae2fc
to
278185a
Compare
go-version: ${{ steps.goversion.outputs.goversion }} | ||
- name: Run Benchmarks | ||
run: | | ||
BENCHSTAT_OUTPUT_FILE=result.txt make test-benchmark-compare REF=release-1.3 |
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.
When we cut branch release-1.4, then we should manually update to REF=release-1.4
on the main branch?
I suggest to add a doc/cut-branch-notes.md
to document all such changes when cutting branch. It can be addressed in a separate PR.
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 can update it, I'll also refactor the workflow real quick to be shared with parameters
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 should ensure we don't creep little percentages over the course of multiple commits into main, compared to the last release branch. Signed-off-by: Thomas Jungblut <[email protected]>
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.
Sorry, this notification went to the bottom of my inbox. Thanks, @tjungblu. LGTM.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, ivanvc, tjungblu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This should ensure we don't creep little percentages over the course of multiple commits into main, compared to the last release branch.