-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add support for manually running the real-time benchmark on PRs #19265
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
base: master
Are you sure you want to change the base?
Conversation
3de527f
to
a233037
Compare
a233037
to
64751de
Compare
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 not currently using this benchmark but if the workflow works well for direct comparisons that seems useful!
required: true | ||
type: string | ||
commit: | ||
description: 'Commit SHA that is going to be benchmarked (e.g. "123456789a")' |
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 not just use ref and baseline_ref? This can be either a branch, tag, commit, etc.
runs-on: ubuntu-22.04 | ||
env: | ||
REPOSITORY: ${{ inputs.repository || 'php/php-src' }} |
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.
These fallbacks seem useless if all inputs are required?
fetch-depth: 100 | ||
path: 'php-version-benchmarks/tmp/php_master' | ||
path: 'php-version-benchmarks/tmp/php_${{ env.BRANCH }}' |
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.
Could probably drop the dynamic path suffix _${{ env.BRANCH }}
here.
@iluuu1994 Thanks for looking into it! I have managed to start testing the manual workflow on my own repo. I implemented some fixes there, but unfortunately a separate issue (github rate limit) blocks it from finishing successfully, so I have to take care of this problem first. Afterwards, I'll update this PR :) |
I couldn't yet test it...