Skip to content
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

CI change, only run bench on performance label #4271

Merged
merged 4 commits into from
Jun 1, 2024

Conversation

soulomoon
Copy link
Collaborator

@soulomoon soulomoon commented May 29, 2024

We already have the performance label

@soulomoon soulomoon added the performance Issues about memory consumption, responsiveness, etc. label May 29, 2024
@soulomoon soulomoon marked this pull request as ready for review May 29, 2024 16:26
@@ -17,6 +17,7 @@ on:
jobs:
pre_job:
runs-on: ubuntu-latest
if: contains(github.event.pull_request.labels.*.name, 'performance')
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite sure if this is enough ?

@soulomoon soulomoon added CI Continuous integration status: needs review This PR is ready for review and removed performance Issues about memory consumption, responsiveness, etc. labels May 29, 2024
@fendor
Copy link
Collaborator

fendor commented May 31, 2024

CI looks like it skipped the benchmark job.

Do we want to run the benchmarks as nightly jobs perhaps? Ideally, there would be an easy way to access them later.

@soulomoon
Copy link
Collaborator Author

soulomoon commented May 31, 2024

CI already skipped bench on some occasions.

And we almost never look at them unless we touch performance related code.

I think for now, if we touch performance related, manually adding the performance label should be enough.

But we can still add the nightly runs, if someone wants it.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@soulomoon soulomoon merged commit 00b6d36 into master Jun 1, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration status: needs review This PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants