Update pre-commit, linters, and labels #28
Conversation
WalkthroughReplaced per-label branch/title blocks in .github/labeler.yml with single pull_request blocks using title regexes for multiple labels and added a new dependencies label scoped to bot authors. Updated GitHub Actions workflows: bumped action versions, adjusted triggers (added push and workflow_dispatch variants), modified permissions, repinned checkout, upgraded QEMU/Buildx, switched to metadata-driven Docker tagging, added build caching, and replaced the version-commit action. Linters workflow now includes push trigger, debug output, updated checkout/setup-python with pip caching, revised caching keys, installs lint requirements, and runs pre-commit-ci as a final step. Bumped merge-conflict labeler action version. Updated .pre-commit-config.yaml (tighter MD excludes, added shellcheck hook, renamed ruff hook id, added ci keys). Added requirements-dev.txt and requirements-lint.txt, removed mypy from requirements.txt, and expanded .gitignore. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks✅ Passed checks (3 passed)
|
Snuffy2
force-pushed
the
Update-pre-commit-and-labels
branch
from
4f8874e to
cf9699e
Compare
Snuffy2
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
.github/workflows/merge_conflict_labeler.yml (1)
18-22: Pin action by commit SHA for supply‑chain safety.
eps1lon/actions-label-merge-conflict@v3.0.3is tag‑pinned. Prefer immutable commit SHAs.-uses: eps1lon/actions-label-merge-conflict@v3.0.3 +# v3.0.3 @ <commit SHA> +uses: eps1lon/actions-label-merge-conflict@0c2f028af2c5aee1caa1b3e7f7b8f1b12f1d9dfdAlso keep permissions minimal (already good). See action docs for usage details. (github.com)
.github/workflows/build_docker.yml (5)
13-18: Least‑privilege: drop unused permissions.
id-token: writeandattestations: writearen’t used. Remove to narrow blast radius.permissions: - contents: write - packages: write - attestations: write - id-token: write + contents: write + packages: write
49-56: Avoid unpinned “@master” action; use a tagged or SHA‑pinned alternative.
actions-js/push@masteris mutable. Use a stable action for committing version bumps.- - name: Commit & Push Version Changes - if: ${{ github.event_name == 'release' && github.event.repository.default_branch == github.event.release.target_commitish && github.event.release.prerelease == false && github.event.release.draft == false }} - uses: actions-js/push@master + - name: Commit & Push Version Changes + if: ${{ github.event_name == 'release' && github.event.repository.default_branch == github.event.release.target_commitish && github.event.release.prerelease == false && github.event.release.draft == false }} + uses: stefanzweifel/git-auto-commit-action@v5 with: - github_token: ${{ secrets.GITHUB_TOKEN }} - branch: ${{ github.event.repository.default_branch }} - message: "Updating to version ${{ github.event.release.tag_name }}" + commit_message: "Updating to version ${{ github.event.release.tag_name }}" + branch: ${{ github.event.repository.default_branch }}Alternatively, pin
actions-js/pushto a specific commit.
57-62: Tag retargeting likely points at the wrong commit.You push a commit via a separate action, but then force‑tag the local, potentially stale, HEAD. Ensure you tag the updated remote commit.
- - name: Update Release with Version Changes Commit + - name: Update Release with Version Changes Commit if: ${{ github.event_name == 'release' && github.event.repository.default_branch == github.event.release.target_commitish && github.event.release.prerelease == false && github.event.release.draft == false }} run: | - git tag -f ${{ github.event.release.tag_name }} - git push -f origin ${{ github.event.release.tag_name }} + git fetch --prune --tags origin + # Point tag to the latest commit on the default branch after the auto-commit + git tag -f ${{ github.event.release.tag_name }} origin/${{ github.event.repository.default_branch }} + git push -f origin refs/tags/${{ github.event.release.tag_name }}
64-74: Tag composition/fallback is fragile; avoid boolean coalescing and commas.
steps.meta.outputs.tagsis newline‑separated; mixing in comma‑separated tags can break parsing. The build action accepts newline lists. (github.com)steps.custom_tags.outputs.tags || steps.meta.outputs.tagsdoes not reliably select a string; GitHub’s expression OR yields booleans unless used in a ternary‑like pattern. Prefer computing a final output explicitly. (docs.github.com)- - name: Add 'latest' tag manually for valid releases + - name: Resolve final tags (adds 'latest' for valid releases) if: ${{ github.event_name == 'release' && github.event.release.prerelease == false && github.event.release.draft == false && github.event.release.target_commitish == github.event.repository.default_branch }} - run: echo "tags=${{ steps.meta.outputs.tags }},ghcr.io/${{ steps.repo-string.outputs.lowercase }}:edge,ghcr.io/${{ steps.repo-string.outputs.lowercase }}:latest" >> "$GITHUB_OUTPUT" - id: custom_tags + id: resolve_tags + run: | + { + echo 'tags<<EOF' + echo "${{ steps.meta.outputs.tags }}" + echo "ghcr.io/${{ steps.repo-string.outputs.lowercase }}:edge" + echo "ghcr.io/${{ steps.repo-string.outputs.lowercase }}:latest" + echo EOF + } >> "$GITHUB_OUTPUT" @@ - tags: ${{ steps.custom_tags.outputs.tags || steps.meta.outputs.tags }} + tags: ${{ steps.resolve_tags.outputs.tags || steps.meta.outputs.tags }}Alternative: let docker/metadata-action set latest automatically via
flavor: latest=trueonly on release events, removing the custom step. (github.com)Also applies to: 89-93, 101-101
44-48: Verify the sed path exists in this repo layout.
./app/const.pymust exist in the container context checked out by this job. If the path is different, the version bump will silently do nothing..github/workflows/linters.yml (1)
48-56: Add explicit job permissions: read‑only.This workflow doesn’t need write permissions. Set minimal permissions at job or workflow level.
jobs: linters: name: Run Linters runs-on: ubuntu-latest + permissions: + contents: read
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.github/labeler.yml(1 hunks).github/workflows/build_docker.yml(5 hunks).github/workflows/linters.yml(1 hunks).github/workflows/merge_conflict_labeler.yml(2 hunks).pre-commit-config.yaml(2 hunks)requirements-dev.txt(1 hunks)requirements-lint.txt(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🔇 Additional comments (10)
requirements-lint.txt (1)
4-10: Reassess stub packages that may now be bundled upstream.
types-setuptools(and some others) can conflict with packages that ship their own typing. If mypy starts emitting duplicate module warnings, drop conflicting stubs..pre-commit-config.yaml (3)
7-10: Anchored Markdown excludes look good.This prevents trimming intentional trailing spaces in Markdown while keeping enforcement elsewhere.
14-17: Good additions: actionlint + shellcheck-py.This pairs well—actionlint will auto-call shellcheck when present. Version pins look current.
Also applies to: 18-26
34-41: Switch to ruff-check aligns with upstream docs.Hook order (check with --fix before format) is correct.
.github/workflows/linters.yml (4)
15-17: Checkout v5 is fine; nothing to change here.
24-32: Setup-python with pip cache is correct.Including
pyproject.tomlin cache keys is helpful when mypy/ruff config lives there.
33-37: Installing lint requirements only: confirm mypy has needed imports.If the project type-checks 3rd‑party imports, you might need runtime packages (not just stubs). Otherwise mypy may mark them as missing or use incomplete stubs.
38-47: Cache pre-commit and mypy: good.Keys include requirements and config; placement before running pre-commit/mypy is correct.
.github/labeler.yml (2)
4-9: Dependencies label by author: valid for srvaroa/labeler.The
authorscondition matches bot authors;appendOnly: trueavoids removing labels later.
10-30: Verified — labeler workflow present and pinned.
Found .github/workflows/labeler.yml — uses: srvaroa/labeler@v1.13.0 (line 13); the labeler config will be applied.
Snuffy2
force-pushed
the
Update-pre-commit-and-labels
branch
from
cf9699e to
74f6444
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/build_docker.yml (2)
47-54: Avoid using a mutable third‑party action ref (master) to push commits.Using actions-js/push@master is a supply‑chain risk. Replace with native git commands and the GITHUB_TOKEN, or pin a specific commit SHA.
Inline replacement (drop the third‑party action):
- - name: Commit & Push Version Changes - if: ${{ github.event_name == 'release' && github.event.repository.default_branch == github.event.release.target_commitish && github.event.release.prerelease == false && github.event.release.draft == false }} - uses: actions-js/push@master - with: - github_token: ${{ secrets.GITHUB_TOKEN }} - branch: ${{ github.event.repository.default_branch }} - message: "Updating to version ${{ github.event.release.tag_name }}" + - name: Commit & Push Version Changes + if: ${{ github.event_name == 'release' && github.event.repository.default_branch == github.event.release.target_commitish && github.event.release.prerelease == false && github.event.release.draft == false }} + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add ./app/const.py + git commit -m "Updating to version ${{ github.event.release.tag_name }}" + git push origin HEAD:${{ github.event.repository.default_branch }}
22-32: Gate debug output behind an input.Same pattern as linters to keep logs clean:
workflow_dispatch: + inputs: + debug: + description: 'Enable extra debug logging' + required: false + type: boolean + default: false ... - name: Debug Variables - run: | + if: ${{ inputs.debug == true }} + run: | echo "github.actor: ${{ github.actor }}" ...
♻️ Duplicate comments (5)
requirements-lint.txt (1)
1-10: Pin lint tool versions or add a constraints file for reproducible CI.Unpinned dev tools will drift and create non‑deterministic runs. Either pin versions directly, or reference a constraints file used by both local and CI.
Option A — add a constraints file reference (recommend):
+-c constraints-lint.txt pre-commit mypy ruff types-cffi types-PyMySQL types-pyRFC3339 types-python-dateutil types-PyYAML types-requests types-setuptoolsCommit constraints-lint.txt with exact pins (generated via pip-compile or freeze).
Option B — minimally bound the top-level tools (repeat similarly for the types-* stubs):
-pre-commit -mypy -ruff +pre-commit>=3.8,<3.9 +mypy>=1.11,<1.12 +ruff>=0.6,<0.7.github/workflows/merge_conflict_labeler.yml (1)
4-7: Scope push trigger to the default branch to avoid no‑op runs.Without PR context, push events will often do nothing but still consume minutes. Restrict to main (or remove push entirely):
on: - push: + push: + branches: + - main pull_request_target: types: - synchronize.github/workflows/linters.yml (1)
18-23: Gate debug echoes behind an explicit input to reduce log noise.workflow_dispatch: + inputs: + debug: + description: 'Enable extra debug logging' + required: false + type: boolean + default: false ... - name: Debug GitHub Variables - run: | + if: ${{ inputs.debug == true }} + run: | echo "github.event_name: ${{ github.event_name }}" echo "github.ref_name: ${{ github.ref_name }}" echo "github.event.repository.default_branch: ${{ github.event.repository.default_branch }}".github/workflows/build_docker.yml (1)
34-41: Pin all actions by commit SHA.checkout, setup-qemu, setup-buildx, metadata-action, login-action, build-push are tag‑pinned. Prefer immutable SHAs for defense‑in‑depth.
Also applies to: 63-64, 74-75, 81-82
requirements-dev.txt (1)
1-2: Share a single dev constraints file across local and CI.Keep toolchain in lockstep by adding a constraints file reference:
+-c constraints-dev.txt -r requirements-lint.txt -r requirements.txtAdd constraints-dev.txt with exact pins (can include or extend constraints-lint.txt).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.github/labeler.yml(1 hunks).github/workflows/build_docker.yml(3 hunks).github/workflows/linters.yml(1 hunks).github/workflows/merge_conflict_labeler.yml(2 hunks).pre-commit-config.yaml(2 hunks)requirements-dev.txt(1 hunks)requirements-lint.txt(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🔇 Additional comments (3)
.pre-commit-config.yaml (1)
18-26: LGTM: shellcheck hook addition is solid.Good coverage alongside actionlint; args [-x] help with sourced scripts. No issues spotted.
.github/workflows/build_docker.yml (1)
56-60: Force‑retagging published releases can break reproducibility.Forcing tags after altering the commit rewrites history consumers might rely on. Ensure this is intentional and documented. Consider creating a new patch tag instead (e.g., vX.Y.Z+meta) or only running the update before the initial publish.
.github/labeler.yml (1)
4-31: Repo uses srvaroa/labeler; appendOnly prevents label removalWorkflows call srvaroa/labeler@v1.13.0 (.github/workflows/labeler.yml) and .github/labeler.yml contains appendOnly: true — this is srvaroa/labeler config (not actions/labeler v5). appendOnly: true makes the action add labels but not remove them; to have labels removed when rules stop matching set appendOnly: false or remove that key. actions/labeler v5 instead exposes a sync-labels input (not a per-label sync). (github.com)
Likely an incorrect or invalid review comment.
Snuffy2
force-pushed
the
Update-pre-commit-and-labels
branch
from
74f6444 to
a57d687
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.github/workflows/build_docker.yml (2)
13-16: Tighten permissions by job/event.contents: write is only needed for the release/tag mutation. Consider splitting into two jobs so the build/push job runs with contents: read and packages: write, and the release job runs with contents: write.
permissions: - contents: write - packages: write + contents: read + packages: writeCreate a separate release-only job with contents: write for the tag push.
17-21: Add job-level concurrency to cancel superseded runs.Prevents stacked builds on rapid pushes.
jobs: build_and_publish_docker_image: + concurrency: + group: docker-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true.github/workflows/merge_conflict_labeler.yml (1)
18-22: Pin action by commit SHA.Hard‑pin eps1lon/actions-label-merge-conflict for supply‑chain hardening.
-uses: eps1lon/actions-label-merge-conflict@v3.0.3 +uses: eps1lon/actions-label-merge-conflict@<SHA>.github/labeler.yml (1)
10-31: Tighten regexes with word boundaries to reduce false positives.Avoid matching substrings like “prefix”.
- title: '(?i).*fix.*' + title: '(?i).*\bfix(es|ed)?\b.*' - title: '(?i).*feat.*' + title: '(?i).*\bfeat(ure)?\b.*' - title: '(?i).*(enhance|improv(e|ement)).*' + title: '(?i).*\b(enhance|improv(e|ement))\b.*' - title: '(?i).*(docs|document|readme).*' + title: '(?i).*\b(docs?|documentation|readme)\b.*' - title: '(?i).*(lint|ruff|mypy|pytest|coverage|type-?check|pre[ -_]?commit).*' + title: '(?i).*\b(lint|ruff|mypy|pytest|coverage|type-?check|pre[ -_]?commit)\b.*' - title: '(?i).*(github[ -_]?actions?|workflow(s)?).*' + title: '(?i).*\b(github[ -_]?actions?|workflow(s)?)\b.*'Optional: add a dependencies title matcher (e.g., chore(deps), bump, deps:).
.github/workflows/linters.yml (1)
54-56: Skip pre-commit-ci lite on non‑PR events.It’s only useful on PRs.
- - uses: pre-commit-ci/lite-action@v1.1.0 - if: always() + - uses: pre-commit-ci/lite-action@v1.1.0 + if: ${{ always() && github.event_name == 'pull_request' }}
♻️ Duplicate comments (7)
.github/workflows/build_docker.yml (1)
39-42: Pin all actions by commit SHA (supply‑chain hardening).All actions are tag‑pinned. Prefer immutable SHAs.
Apply this pattern (replace with the release commit SHA):
-uses: actions/checkout@v5.0.0 +uses: actions/checkout@<SHA> -uses: docker/setup-qemu-action@v3.6.0 +uses: docker/setup-qemu-action@<SHA> -uses: docker/setup-buildx-action@v3.11.1 +uses: docker/setup-buildx-action@<SHA> -uses: docker/metadata-action@v5.8.0 +uses: docker/metadata-action@<SHA> -uses: docker/login-action@v3.5.0 +uses: docker/login-action@<SHA> -uses: docker/build-push-action@v6.18.0 +uses: docker/build-push-action@<SHA>Also applies to: 62-72, 75-76, 81-94, 33-36
requirements-dev.txt (1)
1-2: Consider a dev constraints file for determinism.Reference a constraints file to keep local/CI toolchains in lockstep.
-r requirements-lint.txt -r requirements.txt + -c constraints-dev.txtIf you’d like, I can generate an initial constraints file.
requirements-lint.txt (1)
1-10: Pin lint/tool versions (reproducible CI).Unpinned tools can drift and break CI unexpectedly.
Example:
-pre-commit -mypy -ruff +pre-commit==3.8.0 +mypy==1.11.2 +ruff==0.6.9 -types-cffi +types-cffi==1.16.0.20240331 -types-PyMySQL +types-PyMySQL==1.1.0.20240822 -types-pyRFC3339 +types-pyRFC3339==1.1.0.20240419 -types-python-dateutil +types-python-dateutil==2.9.0.20240906 -types-PyYAML +types-PyYAML==6.0.12.20240808 -types-requests +types-requests==2.32.0.20240914 -types-setuptools +types-setuptools==75.1.0.20240907Or move pins into constraints and add
-c constraints-lint.txt..github/workflows/merge_conflict_labeler.yml (1)
4-7: Limit push trigger to default branch or remove it.Prevents no‑op runs without PR context.
on: - push: + push: + branches: + - main.github/workflows/linters.yml (3)
15-16: Pin actions by commit SHA; add job concurrency.Harden supply‑chain and cancel superseded runs.
jobs: linters: name: Run Linters + concurrency: + group: linters-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true runs-on: ubuntu-latest steps: - - name: Checkout Repository - uses: actions/checkout@v5.0.0 + - name: Checkout Repository + uses: actions/checkout@<SHA> - - name: Setup Python 3 (with caching) - uses: actions/setup-python@v6.0.0 + - name: Setup Python 3 (with caching) + uses: actions/setup-python@<SHA> - - name: Cache pre-commit and mypy - uses: actions/cache@v4.2.4 + - name: Cache pre-commit and mypy + uses: actions/cache@<SHA> - - name: Run pre-commit - uses: pre-commit/action@v3.0.1 + - name: Run pre-commit + uses: pre-commit/action@<SHA> - - uses: pre-commit-ci/lite-action@v1.1.0 + - uses: pre-commit-ci/lite-action@<SHA>Also applies to: 24-32, 38-39, 48-55
26-31: Pin Python minor version for stability.Floating 3.x can change mypy results.
- python-version: 3.x + python-version: '3.12'
18-23: Gate debug logs behind a flag.Reduce log noise in normal runs.
- - name: Debug GitHub Variables - run: | + - name: Debug GitHub Variables + if: ${{ inputs.debug == true || vars.DEBUG == 'true' }} + run: | echo "github.event_name: ${{ github.event_name }}" ...Also add a workflow_dispatch input
debug: falseif desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.github/labeler.yml(1 hunks).github/workflows/build_docker.yml(4 hunks).github/workflows/linters.yml(1 hunks).github/workflows/merge_conflict_labeler.yml(2 hunks).pre-commit-config.yaml(2 hunks)requirements-dev.txt(1 hunks)requirements-lint.txt(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/build_docker.yml
[warning] 51-51: too few spaces before comment: expected 2
(comments)
🔇 Additional comments (6)
.github/workflows/build_docker.yml (2)
33-36: Good: full history fetch for release/tag workflows.fetch-depth: 0 is appropriate here for version bumping and tag operations.
56-61: Force‑pushing tags inside release workflow can be risky.Rewriting the release tag post‑publish may confuse consumers and re-trigger automations.
Do you need the tag rewrite? If the goal is to include the version bump commit, consider creating the commit first and then creating the release from that commit, or use a release‑update action instead of force‑pushing.
.pre-commit-config.yaml (3)
7-10: LGTM: anchored MD excludes.Tighter excludes for Markdown are correct for these hooks.
14-20: Keep actionlint; adding shellcheck is a plus.Nice to have both: actionlint validates GHA syntax and can invoke shellcheck; dedicated shellcheck covers scripts outside workflows.
Ensure shell scripts live under detectable paths (shebang or .sh) so shellcheck runs.
Also applies to: 21-26
34-41: Ruff hook rename is correct.ruff-check with --fix and ruff-format pairing is the current convention.
.github/labeler.yml (1)
4-9: Nice: dependencies label for bot authors.Good coverage of common bot accounts.
Snuffy2
force-pushed
the
Update-pre-commit-and-labels
branch
from
a57d687 to
46453ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
.github/workflows/merge_conflict_labeler.yml (2)
4-5: Limit push trigger to default branch to avoid no‑op runs.On push, there’s no PR context; this workflow will mostly no‑op while still consuming minutes. Restrict it to main (or remove push entirely).
on: - push: + push: + branches: + - main pull_request_target:
6-7: Trigger on more PR events (opened/reopened/edited + synchronize).Labeling only on synchronize can miss fresh PRs or title edits.
pull_request_target: - types: - - synchronize + types: + - opened + - reopened + - edited + - synchronize.github/workflows/build_docker.yml (1)
33-43: Pin all third‑party actions by commit SHA.Harden supply chain by replacing tag pins with immutable SHAs for: checkout, setup‑qemu, setup‑buildx, metadata‑action, login‑action, build‑push‑action.
I can prepare a patch with the exact SHAs for each version in this file.
Also applies to: 62-76, 81-93
requirements-dev.txt (1)
1-2: Consider a shared constraints file for reproducible dev envs.Add a constraints file so local and CI toolchain versions stay in lockstep.
-r requirements-lint.txt -r requirements.txt + -c constraints-dev.txtI can generate an initial constraints file via pip‑compile if you want.
requirements-lint.txt (1)
1-10: Pin lint/tool versions or use constraints for deterministic CI.Unpinned
pre-commit,mypy,ruff, andtypes-*can drift and break CI unexpectedly. Either pin exact versions here or reference a constraints file.Example (pin majors now; exact pins via constraints preferred):
-pre-commit -mypy -ruff +pre-commit~=3.8 +mypy~=1.11 +ruff~=0.6I can propose a constraints file tailored to your current runs.
.github/workflows/linters.yml (2)
15-16: Harden actions, pin Python minor, add least‑privilege perms, and improve cache key.
- Pin all actions by SHA (checkout, setup‑python, cache, pre‑commit/action, pre‑commit‑ci).
- Pin Python to a minor (e.g., '3.12') for stable mypy results. (github.com)
- Add
permissions: contents: readat job level.- Include the Python version in the cache key; add
idto the setup step.jobs: linters: name: Run Linters runs-on: ubuntu-latest + permissions: + contents: read steps: - - name: Checkout Repository - uses: actions/checkout@v5.0.0 + - name: Checkout Repository + uses: actions/checkout@v5.0.0 # replace with commit SHA - - name: Setup Python 3 (with caching) - uses: actions/setup-python@v6.0.0 + - name: Setup Python 3 (with caching) + id: setup-python + uses: actions/setup-python@v6.0.0 # replace with commit SHA with: - python-version: 3.x + python-version: '3.12' cache: 'pip' cache-dependency-path: | requirements-lint.txt pyproject.toml - - name: Cache pre-commit and mypy - uses: actions/cache@v4.2.4 + - name: Cache pre-commit and mypy + uses: actions/cache@v4.2.4 # replace with commit SHA with: path: | ~/.cache/pre-commit .mypy_cache - key: ${{ runner.os }}-lint-${{ hashFiles('**/requirements-lint.txt', '**/.pre-commit-config.yaml', '**/pyproject.toml') }} + key: ${{ runner.os }}-py${{ steps.setup-python.outputs.python-version }}-lint-${{ hashFiles('**/requirements-lint.txt', '**/.pre-commit-config.yaml', '**/pyproject.toml') }} restore-keys: | ${{ runner.os }}-lint- - - name: Run pre-commit - uses: pre-commit/action@v3.0.1 + - name: Run pre-commit + uses: pre-commit/action@v3.0.1 # replace with commit SHA - - uses: pre-commit-ci/lite-action@v1.1.0 + - uses: pre-commit-ci/lite-action@v1.1.0 # replace with commit SHA if: always()Also applies to: 24-31, 38-46, 48-56
18-23: Optional: gate debug echoes to reduce noise.Keep them, but add a DEBUG flag so logs stay clean by default.
- - name: Debug GitHub Variables - run: | + - name: Debug GitHub Variables + if: ${{ vars.DEBUG == 'true' || inputs.debug == true }} + run: | echo "github.event_name: ${{ github.event_name }}" echo "github.ref_name: ${{ github.ref_name }}" echo "github.event.repository.default_branch: ${{ github.event.repository.default_branch }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.github/labeler.yml(1 hunks).github/workflows/build_docker.yml(4 hunks).github/workflows/linters.yml(1 hunks).github/workflows/merge_conflict_labeler.yml(2 hunks).gitignore(1 hunks).pre-commit-config.yaml(2 hunks)requirements-dev.txt(1 hunks)requirements-lint.txt(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🔇 Additional comments (5)
.github/labeler.yml (1)
4-10: Confirm labeler action compatibility with this config schema.This format matches srvaroa/labeler (versioned config with
version: 1,labels,type,title,authors). Ensure your workflow usessrvaroa/labeler(or another action that supports this schema), notactions/labeler(different schema). (github.com)If you want, I’ll scan the repo to verify the labeler action in use and open a follow‑up PR if needed.
.pre-commit-config.yaml (2)
18-26: Shellcheck hook: good addition; type + -x is appropriate.Actionlint + shellcheck combo is set up correctly.
34-41: Ruff hook IDs look correct for recent ruff‑pre‑commit versions.
ruff-check(linter) beforeruff-format(formatter) with--fixis per upstream guidance. (github.com)If you plan to bump Ruff soon, keep the hook IDs aligned with the ruff‑pre‑commit README for that rev.
.github/workflows/build_docker.yml (2)
49-55: [skip ci] will prevent the extra push run; good.Because GitHub Actions natively respects “[skip ci]” on push and pull_request events, the version‑bump commit won’t trigger another run of this workflow’s push branch target. (docs.github.com)
Note: skip tokens don’t apply to pull_request_target workflows.
Also applies to: 56-61
63-73: Metadata tagging configuration looks solid.Use of
latest=auto, semver tags, edge for default branch, and branch ref tags matches docker/metadata‑action guidance. (github.com)
Snuffy2
force-pushed
the
Update-pre-commit-and-labels
branch
from
46453ae to
6f1a291
Compare
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)
.pre-commit-config.yaml (1)
1-6: Optionally set default Python to match CI (stabilize hook envs).Align pre-commit’s Python with CI (e.g., 3.12).
repos: +default_language_version: + python: python3.12Ensure .github/workflows/linters.yml pins 3.12 as well (see my note there).
♻️ Duplicate comments (10)
.github/workflows/build_docker.yml (2)
63-73: Pin all actions by commit SHA.Replace tag pins (checkout, setup-qemu, setup-buildx, metadata, login, build-push, git-auto-commit) with SHAs.
Provide SHAs for the exact versions you want to keep:
Give the immutable commit SHAs for: - actions/checkout@v5.0.0 - docker/setup-qemu-action@v3.6.0 - docker/setup-buildx-action@v3.11.1 - stefanzweifel/git-auto-commit-action@v5 - docker/metadata-action@v5.8.0 - docker/login-action@v3.5.0 - docker/build-push-action@v6.18.0Also applies to: 81-92, 33-43, 49-54, 74-79
22-31: Gate debug echoes behind a flag to reduce log noise.Run only when explicitly enabled.
- - name: Debug Variables - run: | + - name: Debug Variables + if: ${{ vars.DEBUG == 'true' || inputs.debug == true }} + run: | echo "github.actor: ${{ github.actor }}" echo "github.event_name: ${{ github.event_name }}" echo "github.ref_name: ${{ github.ref_name }}" echo "github.event.release.tag_name: ${{ github.event.release.tag_name }}" echo "github.event.repository.default_branch: ${{ github.event.repository.default_branch }}" echo "github.event.release.target_commitish: ${{ github.event.release.target_commitish }}" echo "github.event.release.prerelease: ${{ github.event.release.prerelease }}" echo "github.event.release.draft: ${{ github.event.release.draft }}"requirements-lint.txt (1)
1-10: Pin lint tool and stub versions (or use a constraints file).These float and make CI non-deterministic. Pin exact versions or reference a constraints file.
Apply one of these:
Option A — pin inline:
-pre-commit -mypy -ruff -types-cffi -types-PyMySQL -types-pyRFC3339 -types-python-dateutil -types-PyYAML -types-requests -types-setuptools +pre-commit==3.8.0 +mypy==1.11.2 +ruff==0.6.9 +types-cffi==1.16.0.20240806 +types-PyMySQL==1.1.0.20240821 +types-pyRFC3339==1.1.2.20240106 +types-python-dateutil==2.9.0.20240906 +types-PyYAML==6.0.12.20240917 +types-requests==2.32.0.20240914 +types-setuptools==75.1.0.20240919Option B — use a constraints file:
-pre-commit -mypy -ruff -types-cffi -types-PyMySQL -types-pyRFC3339 -types-python-dateutil -types-PyYAML -types-requests -types-setuptools + -c constraints-lint.txt + pre-commit + mypy + ruff + types-cffi + types-PyMySQL + types-pyRFC3339 + types-python-dateutil + types-PyYAML + types-requests + types-setuptoolsAnd add a new constraints-lint.txt with pinned versions.
I can propose tested pins or generate constraints-lint.txt if you prefer.
requirements-dev.txt (1)
1-2: Share a dev constraints file to keep local/CI in lockstep.Reference a constraints file so toolchain stays stable across environments.
--r requirements-lint.txt --r requirements.txt +-c constraints-dev.txt +-r requirements-lint.txt +-r requirements.txtI can generate constraints-dev.txt (can include or import constraints-lint.txt).
.gitignore (1)
1-29: Add .direnv/ to ignore common local envs.Prevents accidental commits from direnv users.
.yamllint +.direnv/.github/workflows/merge_conflict_labeler.yml (2)
4-7: Restrict push trigger; broaden PR events.Avoid no-op runs on every push; include opened/reopened/edited for reliability.
on: - push: + push: + branches: + - main pull_request_target: - types: - - synchronize + types: + - opened + - reopened + - edited + - synchronize
18-18: Pin the action by commit SHA (supply‑chain hardening).Use immutable SHA for eps1lon/actions-label-merge-conflict.
- uses: eps1lon/actions-label-merge-conflict@v3.0.3 + uses: eps1lon/actions-label-merge-conflict@1df065ebe6e3310545d4f4c4e862e43bdca146f0If you target a newer release, I can fetch its SHA.
.github/labeler.yml (1)
10-30: Tighten regexes to avoid false positives; addchorelabel.Use word boundaries/variants; add chore conventional-commit support.
- title: '(?i).*fix.*' + title: '(?i).*\bfix(e[sd])?\b.*' - title: '(?i).*feat.*' + title: '(?i).*\bfeat(ure)?\b.*' - title: '(?i).*breaking[ -_]change.*' + title: '(?i).*\bbreaking[ -_]?change(s)?\b.*' - title: '(?i).*(github[ -_]?actions?|workflow(s)?).*' + title: '(?i).*\b(github[ -_]?actions?|workflows?)\b.*' - title: '(?i).*(enhance|improv(e|ement)).*' + title: '(?i).*\b(enhance(d|ment)?|improv(e|ement|ed|ing))\b.*' - title: '(?i).*(docs|document|readme).*' + title: '(?i).*\b(docs?|documentation|readme)\b.*' - title: '(?i).*(lint|ruff|mypy|pytest|coverage|type-?check|pre[ -_]?commit).*' + title: '(?i).*\b(lint(ing)?|ruff|mypy|pytest|coverage|type-?check|pre[ -_]?commit)\b.*' + + - label: 'chore' + type: 'pull_request' + title: '(?i)^\s*chore(\b|:)'.github/workflows/linters.yml (2)
18-23: Gate debug echoes to keep logs clean.Run only when debugging.
- - name: Debug GitHub Variables - run: | + - name: Debug GitHub Variables + if: ${{ vars.DEBUG == 'true' || inputs.debug == true }} + run: | echo "github.event_name: ${{ github.event_name }}" echo "github.ref_name: ${{ github.ref_name }}" echo "github.event.repository.default_branch: ${{ github.event.repository.default_branch }}"
15-16: Harden workflow: least privilege, pin Python minor, pin actions by SHA.
- Add job-level permissions: read-only.
- Pin Python to 3.12 to stabilize type checking.
- Replace all tag pins (checkout, setup-python, cache, pre-commit, pre-commit-ci) with SHAs.
jobs: linters: name: Run Linters runs-on: ubuntu-latest + permissions: + contents: read steps: - - name: Checkout Repository - uses: actions/checkout@v5.0.0 + - name: Checkout Repository + uses: actions/checkout@<SHA> @@ - - name: Setup Python 3 (with caching) - uses: actions/setup-python@v6.0.0 + - name: Setup Python 3 (with caching) + uses: actions/setup-python@<SHA> with: - python-version: 3.x + python-version: '3.12' cache: 'pip' @@ - - name: Cache pre-commit and mypy - uses: actions/cache@v4.2.4 + - name: Cache pre-commit and mypy + uses: actions/cache@<SHA> @@ - - name: Run pre-commit - uses: pre-commit/action@v3.0.1 + - name: Run pre-commit + uses: pre-commit/action@<SHA> @@ - - uses: pre-commit-ci/lite-action@v1.1.0 + - uses: pre-commit-ci/lite-action@<SHA>Provide the immutable commit SHAs for: - actions/checkout@v5.0.0 - actions/setup-python@v6.0.0 - actions/cache@v4.2.4 - pre-commit/action@v3.0.1 - pre-commit-ci/lite-action@v1.1.0Also applies to: 24-32, 38-40, 48-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.github/labeler.yml(1 hunks).github/workflows/build_docker.yml(4 hunks).github/workflows/linters.yml(1 hunks).github/workflows/merge_conflict_labeler.yml(2 hunks).gitignore(1 hunks).pre-commit-config.yaml(2 hunks)requirements-dev.txt(1 hunks)requirements-lint.txt(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (1)
- requirements.txt
🔇 Additional comments (1)
.github/workflows/build_docker.yml (1)
90-91: LGTM: cache strategy is consistent.cache-from/to both unscoped type=gha; no mismatched scopes.
This pull request updates several GitHub workflow and configuration files to improve CI/CD automation, dependency management, and code quality enforcement. The main changes include modernizing workflow actions, refining label automation, enhancing caching and dependency handling for linting, and updating pre-commit hooks for better shell and Python code quality checks.
CI/CD Workflow Modernization & Improvements:
.github/workflows/build_docker.ymlto use specific versioned actions for better reliability and reproducibility, improved Docker tag generation, added build cache support, and switched the commit/push step to a more stable action (stefanzweifel/git-auto-commit-action@v5). [1] [2] [3]main, improved caching for Python and linting dependencies, and enhanced debug output in.github/workflows/linters.yml..github/workflows/merge_conflict_labeler.yml. [1] [2]Label Automation Refinements:
.github/labeler.ymlby consolidating label matching rules, adding author-based dependency labeling, and making title matching more robust for various PR types.Pre-commit & Linting Enhancements:
shellcheck-pyto.pre-commit-config.yamlfor shell script linting, clarified exclusion patterns for markdown files, and updated Ruff hook usage for clarity and accuracy. [1] [2]requirements-lint.txtand updatedrequirements-dev.txtto reference both linting and main requirements, ensuring all necessary packages are installed for code quality checks. [1] [2]