ci(workflows): migrate ci.yml to dorny/paths-filter detect job (#413) - #460
ci(workflows): migrate ci.yml to dorny/paths-filter detect job (#413)#460yoshi280 wants to merge 1 commit into
Conversation
… detect job (#413) paths-ignore made `pytest 3.12/3.13` register as `expected` (not satisfied) on docs-only PRs, blocking merge under branch protection that requires those checks. Surfaced concretely by #426 where the README v1.7-row update could not merge despite being approved and non-Python. Replace with the pattern documented in the prior TODO comment: workflow runs on every PR; a detect job uses dorny/paths-filter to classify whether the diff touches Python source / tests / deps; the pytest job gates on `if: needs.detect.outputs.python == 'true'`. Skipped jobs register as `success` to branch protection — gating the work, not the registration. dorny/paths-filter pinned at v3.0.2 SHA per repo SHA-pin convention (harden-runner / setup-uv pinned similarly upstream).
Reviewer's GuideMigrates the CI workflow from using pull_request.paths-ignore to a dedicated detection job using dorny/paths-filter, so the CI workflow always runs and pytest is conditionally skipped via an if-gated job instead of being unregistered for docs-only changes. Sequence diagram for pull request triggering detect and conditionally running pytestsequenceDiagram
actor Developer
participant GitHub
participant CI_Workflow
participant Job_detect
participant Paths_filter
participant Job_pytest
Developer->>GitHub: Open or update pull_request targeting main
GitHub-->>CI_Workflow: Trigger CI workflow (no paths-ignore)
CI_Workflow->>Job_detect: Start detect job
Job_detect->>Paths_filter: Run dorny/paths-filter with python filters
Paths_filter-->>Job_detect: outputs.python (true or false)
Job_detect-->>CI_Workflow: Job outputs python = steps.filter.outputs.python
CI_Workflow->>Job_pytest: Evaluate needs.detect.outputs.python
alt Changes touch Python src/tests/deps
Job_pytest-->>CI_Workflow: Condition true, run pytest job
else Docs-only or non-Python changes
Job_pytest-->>CI_Workflow: Condition false, job skipped (reported as skipped)
end
CI_Workflow-->>GitHub: Report status for detect and pytest checks
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="27-28" />
<code_context>
jobs:
+ detect:
+ runs-on: ubuntu-latest
+ outputs:
+ python: ${{ steps.filter.outputs.python }}
+ steps:
+ - uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against missing or malformed `paths-filter` output to avoid surprising job skips.
This relies on `steps.filter.outputs.python` always being set to `'true'` or `'false'`. If the `dorny/paths-filter` step is renamed, skipped, or fails, `needs.detect.outputs.python` becomes empty and `pytest` will never run. Consider normalizing the value through an intermediate step/job-level output that defaults to `'true'` when unset, so misconfigurations still run pytest rather than silently skipping it.
Suggested implementation:
```
detect:
runs-on: ubuntu-latest
outputs:
python: ${{ steps.normalize-python.outputs.python }}
```
```
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: filter
with:
filters: |
python:
- 'src/**'
- 'tests/**'
- name: Normalize python filter output
id: normalize-python
env:
PYTHON_MATCH: ${{ steps.filter.outputs.python }}
run: |
if [ "${PYTHON_MATCH}" = "true" ] || [ "${PYTHON_MATCH}" = "false" ]; then
echo "python=${PYTHON_MATCH}" >> "$GITHUB_OUTPUT"
else
# Default to true if the filter output is missing or malformed
echo "python=true" >> "$GITHUB_OUTPUT"
fi
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| outputs: | ||
| python: ${{ steps.filter.outputs.python }} |
There was a problem hiding this comment.
suggestion (bug_risk): Guard against missing or malformed paths-filter output to avoid surprising job skips.
This relies on steps.filter.outputs.python always being set to 'true' or 'false'. If the dorny/paths-filter step is renamed, skipped, or fails, needs.detect.outputs.python becomes empty and pytest will never run. Consider normalizing the value through an intermediate step/job-level output that defaults to 'true' when unset, so misconfigurations still run pytest rather than silently skipping it.
Suggested implementation:
detect:
runs-on: ubuntu-latest
outputs:
python: ${{ steps.normalize-python.outputs.python }}
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: filter
with:
filters: |
python:
- 'src/**'
- 'tests/**'
- name: Normalize python filter output
id: normalize-python
env:
PYTHON_MATCH: ${{ steps.filter.outputs.python }}
run: |
if [ "${PYTHON_MATCH}" = "true" ] || [ "${PYTHON_MATCH}" = "false" ]; then
echo "python=${PYTHON_MATCH}" >> "$GITHUB_OUTPUT"
else
# Default to true if the filter output is missing or malformed
echo "python=true" >> "$GITHUB_OUTPUT"
fi
|
Superseded by #446, which shipped the dorny/paths-filter migration to Closing rather than rebasing to avoid the duplicate-implementation churn. |
2 participants
Re-open of #442 (auto-closed during a rebase attempt that hit a real merge conflict in
.github/workflows/ci.yml; the branch was force-pushed to main's HEAD, then restored to its original commit60387b3). The original PR could not be reopened via API.Refers #413. The diff is the original single signed commit. Author should resolve the merge conflict in ci.yml against current main (edge-rerank ship modified overlapping region) before this can be merged.
Summary by Sourcery
CI: