-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Only run asv benchmark when labeled #5893
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
Conversation
${{ contains( github.event.pull_request.labels.*.name, 'run-benchmark') | ||
&& github.event_name == 'pull_request' | ||
|| github.event_name == 'workflow_dispatch' }} | ||
if: ${{ contains( github.event.pull_request.labels.*.name, 'run-benchmark') && github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' }} |
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.
would it make sense to alternatively allow setting a tag in the commit message? I think it's more likely someone will need to test the performance of specific commits rather than the whole PR. We could even extend ci-trigger
to optionally detect labels on PRs...
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.
Well, I think adding or removing a label is much easier than to remember to add a tag in the commit message. I rarely get it right the first (or second) commit anyway so remembering to add it in several messages is a tough task. The benchmark takes about the same time as the other tests now so I don't think it won't be that much of a bottleneck if it runs a few more times than necessary.
Adding labels is however an admin-only thing so if other contributors want's to run it without asking the maintainers a tag in the commit is good a option.
* upstream/main: Only run asv benchmark when labeled (pydata#5893) Add asv benchmark jobs to CI (pydata#5796) Remove use of deprecated `kind` argument in `CFTimeIndex` tests (pydata#5723) Single matplotlib import (pydata#5794) Check jupyter nbs with black in pre-commit (pydata#5891)
* Only run benchmark when labeled * Update benchmarks.yml * Update benchmarks.yml
Small fix to #5796. The benchmark was only intended to run when the PR has the label
run-benchmark
. I split the if condition in multiple lines for better readability thinking it didn't change the function but it did.