[TEST] No-trigger test: .md file change - #3009
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mergify
Bot
added
documentation
Merge Protections🔴 2 of 2 protections blocking · waiting on 👀 reviews
🔴 Require one maintainer reviewWaiting for any of
This rule is failing.All PRs must have at least one approving review from a maintainer before merging.
🔴 Require two reviewsWaiting for
This rule is failing.PRs labelled "two-reviews" must have at least two approving reviews before merging.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Buildkite pipeline configurations (gpu-tests-H100.yml and gpu-tests-L4.yml) to split the "Transformers Tests" step into two separate steps: one that always runs on branch pushes, and another that runs conditionally on pull requests when relevant files are modified. Feedback on these changes highlights critical Buildkite syntax issues in both YAML files. Specifically, build.pull_request evaluates to false rather than null on branch builds, which breaks the conditional logic. Additionally, Buildkite's if_changed attribute does not support nested include and exclude keys; instead, a flat list of glob patterns using the ! prefix should be used for exclusions to prevent pipeline parsing errors.
| - label: ":mag: Detect changes for Transformers Tests" | ||
| # Transformers tests - always run on branch pushes | ||
| - label: ":mag: Transformers Tests (branch)" | ||
| if: build.pull_request == null |
There was a problem hiding this comment.
In Buildkite, build.pull_request evaluates to false (boolean) when the build is not triggered by a pull request, rather than null. Using build.pull_request == null will evaluate to false on branch builds, preventing this step from running.
Please use build.pull_request == false instead.
if: build.pull_request == false| - label: ":mag: Detect changes for Transformers Tests" | ||
| # Transformers tests - always run on branch pushes | ||
| - label: ":mag: Transformers Tests (branch)" | ||
| if: build.pull_request == null |
There was a problem hiding this comment.
In Buildkite, build.pull_request evaluates to false (boolean) when the build is not triggered by a pull request, rather than null. Using build.pull_request == null will evaluate to false on branch builds, preventing this step from running.
Please use build.pull_request == false instead.
if: build.pull_request == false|
|
||
| # Transformers tests - conditional on PRs when relevant paths changed | ||
| - label: ":mag: Transformers Tests (PR)" | ||
| if: build.pull_request != null |
There was a problem hiding this comment.
In Buildkite, build.pull_request evaluates to false (boolean) when the build is not triggered by a pull request. Using build.pull_request != null will evaluate to true on branch builds (since false != null is true), causing this PR-only step to run on branch pushes as well.
Please use build.pull_request != false instead.
if: build.pull_request != false| if_changed: | ||
| include: | ||
| - "src/**" | ||
| - "tests/**" | ||
| - "setup.py" | ||
| - "MANIFEST.in" | ||
| - ".buildkite/gpu-tests/**" | ||
| exclude: | ||
| - "tests/e2e/**" | ||
| - "tests/lmeval/**" | ||
| - "tests/examples/**" | ||
| - "**/*.md" |
There was a problem hiding this comment.
Buildkite's native if_changed attribute expects a single glob pattern or an array of glob patterns. It does not support nested include and exclude keys, which will result in a pipeline parsing error.
To exclude specific paths, you can use the ! prefix directly within a flat list of glob patterns.
if_changed:
- "src/**"
- "tests/**"
- "setup.py"
- "MANIFEST.in"
- ".buildkite/gpu-tests/**"
- "!tests/e2e/**"
- "!tests/lmeval/**"
- "!tests/examples/**"
- "!**/*.md"
|
👋 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 complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
42c18ca to
d27327f
Compare
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
… regex operator Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
d27327f to
4fdd529
Compare
1 participant
Test PR to verify if_changed exclusions
This PR modifies a
.mdfile which should NOT TRIGGER transformers tests (excluded).Part of verifying the if_changed include/exclude pattern in #2983