-
Notifications
You must be signed in to change notification settings - Fork 30
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
Set paths to not run on unrelated PRs #370
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,10 @@ name: Llama Benchmarking Tests | |
on: | ||
workflow_dispatch: | ||
pull_request: | ||
paths: | ||
- '.github/workflows/ci-llama.yaml' | ||
- 'sharktank/**' | ||
- '*requirements*.txt' | ||
schedule: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left a comment on #363 (comment). I'd rather see this run on either The workflow name also isn't consistent with others ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sense. In that case, lets wait for @aviator19941 to respond to your comment and then refactor accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to have more models to test and benchmark, so I'd like us to get better organized about how we define workflows. |
||
# Weekdays at 5:00 AM UTC = 10:00 PM PST. | ||
- cron: "0 5 * * 1-5" | ||
|
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 for the moment we should actually remove all these path filters from the workflows and start switching more checks to be required. Too many developers are ignoring CI results and relying on auto-merge, wasting multiple developer-hours a week on reverts and fix-forwards.
We could build a system like https://iree.dev/developers/general/github-actions/#opt-in-for-presubmit-jobs to get the best of both worlds (filtering and path filters), since GitHub's default implementation is completely broken :|
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 generally agree, but don't have a good feeling yet which workflows would be a good choice to be run on every PR. However, I clearly see the point. Closing this therefore.
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.
Every workflow that runs in less than 30 minutes.