Group GitHub workflow definitions by language#631
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoved a combined code-quality workflow, renamed and narrowed the Python workflow/job, removed the Rust job from that file, and added a separate Rust-only workflow that runs Flox/Mask-based Rust checks on push and pull_request. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Push/PR (Python changes)
participant GH as GitHub Actions
participant PY as "Python code checks"
participant Flox as Flox/Mask
Dev->>GH: push/pull_request
GH->>PY: start workflow
PY->>PY: checkout repo
PY->>Flox: install Flox
PY->>Flox: activate & run Python checks
PY-->>GH: report status
sequenceDiagram
autonumber
actor Dev as Push/PR (Rust changes)
participant GH as GitHub Actions
participant RS as "Rust code checks"
participant Flox as Flox/Mask
Dev->>GH: push/pull_request
GH->>RS: start workflow
RS->>RS: checkout repo
RS->>Flox: install Flox
RS->>Flox: activate & run `mask development rust all`
RS-->>GH: report status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
There was a problem hiding this comment.
Pull Request Overview
Restructures GitHub workflow organization by separating Rust and Python code checks into dedicated workflow files with path-based triggers to improve CI efficiency and maintainability.
- Split previously combined workflows into language-specific files
- Added path filters to trigger workflows only when relevant language files change
- Consolidated testing and quality checks within each language workflow
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/run_rust_code_checks.yaml |
New workflow file for Rust code checks with path filtering |
.github/workflows/run_python_code_checks.yaml |
Updated workflow to include path filtering and renamed for consistency |
.github/workflows/check_code_quality.yaml |
Removed file - functionality moved to language-specific workflows |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/run_python_code_checks.yaml (1)
23-27: Fix Coveralls upload: use Cobertura inputs for .xml or supply LCOVpath-to-lcov is the legacy input for LCOV (.info); the workflow currently points to coverage_output/.python_coverage.xml (Cobertura XML). Change to one of the options below.
File: .github/workflows/run_python_code_checks.yaml — lines 23–27
Option A — keep Cobertura XML:
- - name: Upload coverage to Coveralls - uses: coverallsapp/github-action@v2 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: coverage_output/.python_coverage.xml + - name: Upload coverage to Coveralls + uses: coverallsapp/github-action@v2 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + format: cobertura + file: coverage_output/.python_coverage.xmlOption B — provide LCOV:
- path-to-lcov: coverage_output/.python_coverage.xml + path-to-lcov: coverage_output/lcov.info
🧹 Nitpick comments (4)
.github/workflows/run_rust_code_checks.yaml (2)
5-9: Broaden path filters to include Cargo/config changes (ensure checks run when config changes).Only matching “**.rs” will miss Cargo.toml/lock, toolchain, Maskfile, and workflow edits that affect Rust checks. Recommend expanding.
on: push: paths: - - '**.rs' + - '**/*.rs' + - 'Cargo.toml' + - 'Cargo.lock' + - 'rust-toolchain*' + - 'mask*' + - '.github/workflows/run_rust_code_checks.yaml' pull_request: paths: - - '**.rs' + - '**/*.rs' + - 'Cargo.toml' + - 'Cargo.lock' + - 'rust-toolchain*' + - 'mask*' + - '.github/workflows/run_rust_code_checks.yaml'
1-3: Add minimal permissions, concurrency cancelation, and a job timeout.Safer defaults and faster CI for rapid pushes.
name: Rust code checks +permissions: + contents: read on: @@ jobs: run_rust_code_checks: name: Run Rust code checks runs-on: ubuntu-latest + timeout-minutes: 30 + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: trueAlso applies to: 10-14
.github/workflows/run_python_code_checks.yaml (2)
5-9: Include non-.py config files in path filters (pyproject, requirements, linters).Otherwise Python checks won’t run when only tooling/config changes.
on: push: paths: - - '**.py' + - '**/*.py' + - 'pyproject.toml' + - 'poetry.lock' + - 'requirements*.txt' + - 'setup.cfg' + - 'setup.py' + - '.flake8' + - '.ruff.toml' + - 'mypy.ini' + - '.github/workflows/run_python_code_checks.yaml' pull_request: paths: - - '**.py' + - '**/*.py' + - 'pyproject.toml' + - 'poetry.lock' + - 'requirements*.txt' + - 'setup.cfg' + - 'setup.py' + - '.flake8' + - '.ruff.toml' + - 'mypy.ini' + - '.github/workflows/run_python_code_checks.yaml'
15-22: Pin action versions, set minimal permissions, add timeout and concurrency.Harden workflow and speed up subsequent pushes.
name: Python code checks +permissions: + contents: read @@ run_python_code_checks: name: Run Python code checks runs-on: ubuntu-latest + timeout-minutes: 45 + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true @@ - - name: Checkout code - uses: actions/checkout@v4 + - name: Checkout code + uses: actions/checkout@v4 # pin to a commit SHA @@ - - name: Install Flox - uses: flox/install-flox-action@v2 + - name: Install Flox + uses: flox/install-flox-action@v2 # pin to a commit SHA @@ - - name: Run unit tests with coverage - uses: flox/activate-action@v1 + - name: Run unit tests with coverage + uses: flox/activate-action@v1 # pin to a commit SHANote: If Coveralls needs to post PR comments/checks, you may need to add:
permissions: contents: read pull-requests: write checks: writePlease adjust based on the action’s docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/check_code_quality.yaml(0 hunks).github/workflows/run_python_code_checks.yaml(1 hunks).github/workflows/run_rust_code_checks.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/check_code_quality.yaml
🔇 Additional comments (3)
.github/workflows/run_python_code_checks.yaml (2)
11-12: LGTM on the split and rename.Python and Rust checks split by language with clear job names improves readability and CI focus.
19-22: Coverage output path verified — mask writes coverage to coverage_output/.python_coverage.xmlpyproject.toml sets [tool.coverage.xml] output = "coverage_output/.python_coverage.xml"; tests.yaml runs
coverage xml -o /tests/coverage_output/.python_coverage.xmlwith./coverage_outputmounted to/tests/coverage_output; maskfile.md createscoverage_output. No change needed..github/workflows/run_rust_code_checks.yaml (1)
20-22: Verify Mask task availability and failure surface.
- Confirmed: maskfile.md defines the development → rust → all task; the 'all' target invokes
mask development rust check/format/lint/test(see maskfile.md under "## development" → "### rust" → "#### all").- Outstanding: cannot verify that flox/activate-action@v1 supplies the Mask CLI or that the action will fail the job on failing lints/tests — ensure the runner installs/provides
maskor add a pre-check (e.g.,mask --version) / explicit install step so failures surface as job failures.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Overview
Changes
Comments
This is a more idiomatic way to do it. I think this should get us what we want and it mimics what's in the pre-commit hooks.
Summary by CodeRabbit