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

Require 'ready' label for transformers tests #1079

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

dbarbuzzi
Copy link
Collaborator

SUMMARY:
This PR moves the transformers-tests job to its own workflow which only runs on a PR that has the 'ready' label. With this implementation, if the PR already has the 'ready' label when a new commit is pushed, the new commit will re-run the job. I also updated the PR comment workflow to note that the 'ready' label must be added to run this job.

There are some visual warts/caveats that come with these triggers because the filtering cannot be done by the triggers, only the jobs:

Whenever a label is added or a commit is pushed, the workflow will still be triggered. However, if the PR does not have the 'ready' label, the transformers-tests job will be skipped. While the job being skipped means the runner/resources will not actually be used, it also means that the workflow/job will still show up in various places in the GitHub UI (Actions tab, PR checks). It will show as skipped, but it will still allow the green check mark because that appears as long as there are no failures.

Basically, this means that reviews/people able to merge will need to pay closer attention to the jobs run on the PR to make sure that the transformers-tests job was run/not skipped.

TEST PLAN:
Should affect this PR.

Signed-off-by: Domenic Barbuzzi <domenic@neuralmagic.com>
Copy link

👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review.

Note: This is required to trigger the transformers tests.

@dbarbuzzi
Copy link
Collaborator Author

dbarbuzzi commented Jan 17, 2025

Commenting here: At this stage, all workflows/jobs except the transformers-tests job have run (and the bot PR comment is the updated version with the new note).

@dbarbuzzi dbarbuzzi added the ready When a PR is ready for review label Jan 17, 2025
@dbarbuzzi
Copy link
Collaborator Author

Commenting after adding the 'ready' label: The transformers-tests workflow/job is now queued after adding the label.

dsikka
dsikka previously approved these changes Jan 17, 2025
Copy link
Collaborator

@dsikka dsikka left a comment

Choose a reason for hiding this comment

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

LGTM. One comment about the note we're providing to users

.github/workflows/set-comment.yaml Outdated Show resolved Hide resolved
Signed-off-by: Domenic Barbuzzi <domenic@neuralmagic.com>
Copy link
Collaborator

@rahul-tuli rahul-tuli left a comment

Choose a reason for hiding this comment

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

LGTM!

@dsikka dsikka merged commit b175943 into main Jan 17, 2025
6 of 7 checks passed
@dsikka dsikka deleted the ready-label-for-transformers-tests branch January 17, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants