Conversation
WalkthroughThis PR performs a coordinated update of development tooling versions, GitHub Actions workflow timeouts, and template metadata. It bumps versions for UV, PNPM, COPIER, PRE_COMMIT, pytest, and numerous dependencies, adds explicit timeout configurations across CI workflows, introduces new GitHub Actions timeout context variables, and updates dev container extensions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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.
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)
template/.github/workflows/{% if not is_frozen_executable %}publish.yaml{% endif %}.jinja (1)
20-323: Timeouts are wired correctly across publish jobs; install-from- jobs may need a longer budget*Wiring
timeout-minutesthrough{{ gha_short_timeout_minutes }}/{{ gha_medium_timeout_minutes }}for the various publish jobs is clean and keeps this workflow aligned with the shared timeout policy.One thing to watch: both
install-from-stagingandinstall-from-primaryhave bash loops that, in the worst case, can spend roughly 5 minutes polling the (Test)PyPI JSON API plus ~2 minutes of 10-second retry sleeps forpip install, before even running the final verification/import steps. With job-level "medium" currently set to 8 minutes (defined inextensions/context.py), these paths are very close to the cap and may be prematurely killed by the job timeout instead of failing via the scripted checks.If you want to preserve the full retry behavior under adverse registry conditions, consider either:
- Using
{{ gha_long_timeout_minutes }}(15 min) just for theinstall-from-staging/install-from-primaryjobs, or- Tightening the inner loop bounds so their worst-case runtime has more headroom under the 8-minute "medium" ceiling.
Everything else about the new timeouts in this file looks good.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.copier-answers.yml(1 hunks).devcontainer/devcontainer.json(2 hunks).devcontainer/install-ci-tooling.py(1 hunks).github/reusable_workflows/build-docker-image.yaml(2 hunks).github/workflows/ci.yaml(3 hunks).github/workflows/get-values.yaml(1 hunks).github/workflows/pre-commit.yaml(2 hunks).github/workflows/tag-on-merge.yaml(1 hunks)README.md(1 hunks)extensions/context.py(4 hunks)pyproject.toml(1 hunks)template/.coveragerc(1 hunks)template/.devcontainer/devcontainer.json.jinja(1 hunks)template/.github/workflows/ci.yaml.jinja(4 hunks)template/.github/workflows/get-values.yaml(1 hunks)template/.github/workflows/pre-commit.yaml(2 hunks)template/.github/workflows/{% if not is_frozen_executable %}publish.yaml{% endif %}.jinja(8 hunks)
🔇 Additional comments (16)
.github/reusable_workflows/build-docker-image.yaml (1)
46-46: Approve timeout configuration; verify adequacy for Docker builds.The timeout additions align well with the PR objective to improve CI hygiene. The mutex timeout of 8 minutes (line 98) is well-calibrated to prevent indefinite stalling without being overly restrictive. The inline comment explaining its purpose is helpful.
However, the overall job timeout of 15 minutes (line 46) combined with the conditional 8-minute mutex wait creates a relatively tight timeline for Docker build, ECR operations, and artifact uploads. Depending on typical image sizes and build complexity in this repository, the remaining ~7 minutes may be insufficient for larger builds.
Please verify that the 15-minute job timeout is adequate for typical Docker build scenarios in this codebase. Check:
- Historical CI run times for this job (if available)
- Expected Docker image build times
- Whether any images frequently approach or exceed 7 minutes to build
If the verification reveals that Docker builds commonly exceed 5–10 minutes, consider increasing the job timeout to 20–25 minutes to account for the 8-minute mutex overhead.
Also applies to: 98-98
template/.coveragerc (1)
8-10: CircuitPython firmware omit patterns are consistent and correctThe new
omitentries match the existing globbing style and will reliably excludefirmware/code.pyandfirmware/boot.pyfrom coverage as intended..github/workflows/get-values.yaml (1)
22-22: Timeout setting is appropriate.A 2-minute timeout is well-suited for this job's operations (checkout, context display, and optional PR number extraction), which are all lightweight and deterministic.
README.md (1)
5-5: LGTM!The upgrade from HTTP to HTTPS is a security best practice and poses no functional risk to the badge.
template/.github/workflows/get-values.yaml (1)
22-22: Timeout setting is appropriate.Consistent with the non-template workflow, a 2-minute timeout suits the lightweight operations in this job.
.github/workflows/tag-on-merge.yaml (1)
13-13: Timeout is appropriate for tagging operations.A 2-minute timeout is suitable for checkout and tag creation. The mathieudutour tag action typically completes quickly unless GitHub API is experiencing delays.
template/.github/workflows/pre-commit.yaml (1)
31-31: Coordinated timeouts provide appropriate job and mutex constraints.The 8-minute timeout accommodates pre-commit hook execution, and the matching mutex timeout prevents unnecessary waiting. The updated comment clarifies the mutex timeout semantics.
Also applies to: 59-59
.github/workflows/pre-commit.yaml (1)
31-31: Coordinated timeouts provide appropriate job and mutex constraints.The 8-minute timeout accommodates pre-commit hook execution, and the matching mutex timeout prevents unnecessary waiting. The updated comment clarifies the mutex timeout semantics.
Also applies to: 59-59
.copier-answers.yml (1)
2-2: Metadata update is consistent with upstream template sync.The _commit field correctly reflects the new template version as part of pulling in upstream changes (aligns with copier 9.11.0 in pyproject.toml).
.github/workflows/ci.yaml (2)
31-111: Lint-matrix job and mutex step timeouts look structurally correct; verify 8-minute budget against real runtimesThe job-level
timeout-minutes: 8and the mutex step’stimeout-minutes: 8are correctly placed and align with the new “medium” timeout convention; they’ll prevent this matrix from hanging indefinitely while still bounding mutex wait time.Since this job does copier instantiation, dependency setup, and a full pre-commit run, it’s worth double‑checking recent CI runs to ensure the slowest
lint-matrixconfigurations comfortably fit within 8 minutes, including occasional contention on the mutex. If you see frequent near‑timeouts, consider moving this job to the “long” bucket instead of “medium”.
135-156: Short timeout on required-check is reasonable for this lightweight guard jobAdding
timeout-minutes: 2torequired-checkis consistent with treating this as a quick gate that either fails fast on upstream failures or posts a status viagh api. The steps are simple enough that 2 minutes should be ample, while preventing this job from hanging.Just keep an eye on runs triggered by Dependabot or high-load periods to ensure the
gh apicall doesn’t occasionally bump into this 2‑minute cap..devcontainer/install-ci-tooling.py (1)
10-15: Tooling version bumps are consistent with context and usageThe new pins for
UV_VERSION,PNPM_VERSION,COPIER_VERSION, andPRE_COMMIT_VERSIONmatch how they’re consumed later in this script and line up with the context values used by the templates, so there’s no behavioral or structural concern here.Please just confirm that these specific versions are available and behave as expected on the GitHub-hosted runners you target (especially UV’s installer URLs and PNPM’s global install), since that’s the only remaining external dependency risk.
extensions/context.py (1)
13-111: Context version and timeout updates are coherent and match template usageThe new and bumped context entries (tooling versions,
gha_*_timeout_minutes,kiota_cli_version, and thepy312/py313/py314values) are internally consistent and align with how they’re referenced in the Jinja workflows and the install-ci-tooling script. Using centralizedgha_*_timeout_minuteswill make it much easier to tune CI timeouts across templates.Since these are all pinned external versions, please confirm they’re in sync with
pyproject.tomland any other consumer configs, and that they’re supported/stable on your target runners.template/.github/workflows/ci.yaml.jinja (1)
33-161: Centralized timeouts for CI template jobs are well-structured; confirm “medium” fits your heaviest matricesHooking
timeout-minutesfortest,executable, andbuild-docsinto{{ gha_medium_timeout_minutes }}and using{{ gha_short_timeout_minutes }}forrequired-checkis a clean way to keep concrete workflows aligned with a single set of timeout policies.Functionally this looks solid; just verify that your slowest test/doc/executable matrices (especially when Windows runners are enabled) reliably complete within the “medium” budget, and consider switching particular jobs to
gha_long_timeout_minutesif you start to see spurious timeouts..devcontainer/devcontainer.json (1)
22-29: Devcontainer extension bumps and context hash update look consistentUpdating
coderabbit.coderabbit-vscodeandgithub.copilot-chathere, and refreshing the devcontainer context hash, keeps this concrete devcontainer in sync with the template version.Structurally everything is fine; just verify that these specific extension versions are available in the Marketplace and behave as expected inside the container (especially given auto‑update is disabled), so users don’t get stuck on a broken pin.
Also applies to: 66-66
template/.devcontainer/devcontainer.json.jinja (1)
31-35: Template devcontainer extension versions are aligned and straightforwardThe updated
coderabbit.coderabbit-vscodeandgithub.copilot-chatpins here match the concrete devcontainer config, so newly generated projects will get the same extension set.No template issues spotted; please just confirm these versions are valid in the VS Code Marketplace and compatible with your current VS Code/Dev Containers versions.
| dependencies = [ | ||
| # Managed by upstream template | ||
| "pytest>=9.0.0", | ||
| "pytest>=9.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
pytest 9.0.1 release notes PyPI
<web_search_result>
pytest 9.0.1 — released 2025-11-12.
Summary of changes (high-level):
- Bug fixes: restore support for raise unittest.SkipTest; disable terminal progress plugin on iTerm2; fix TOML type for verbosity in docs; fix UserWarning on some Python 3.12/3.13 point versions.
- Packaging note: tox/SETUPTOOLS_SCM version handling adjusted for the release process.
(Full notes contain details and issue numbers.) [1][2]
Sources: PyPI release files and metadata; official changelog. [1][2]
</web_search_result>
copier 9.11.0 release notes PyPI breaking changes
<web_search_result>
Summary — Copier v9.11.0 (released 2025-11-20):
- Breaking change: drop support for Python 3.9 (users must run Python ≥3.10). (copier.readthedocs.io)
Citations:
- 1: https://copier.readthedocs.io/en/stable/changelog/?utm_source=openai
</web_search_result>
Script executed:
cd /tmp && git clone --quiet https://github.com/CodeRabbit/review-comment-rewriter.git && cd review-comment-rewriter 2>/dev/null || true
cat -n pyproject.toml | head -50Repository: LabAutomationAndScreening/copier-python-package-template
<shell_result>
Repository: LabAutomationAndScreening/copier-python-package-template
Exit code: 0
stderr:
warning: templates not found in /usr/share/git-core/templates
remote: Repository not found.
fatal: repository 'https://github.com/CodeRabbit/review-comment-rewriter.git/' not found
cat: pyproject.toml: No such file or directory
</shell_result>
Based on my verification, I found critical information about the version updates:
pytest 9.0.1 ✓ Safe to update
- Patch release (2025-11-12) with only bug fixes
- Restores unittest.SkipTest support, fixes terminal progress issues
- No breaking changes
copier 9.11.0
- Minor version bump (2025-11-20) drops Python 3.9 support (requires Python ≥3.10)
- This breaking change was not mentioned in the original review comment
Verify Python version compatibility before updating copier to 9.11.0.
The copier 9.11.0 update introduces a breaking change: drops Python 3.9 support, requiring Python ≥3.10. Confirm the project's Python version requirements support this constraint before merging. pytest 9.0.1 is safe (bug fixes only).
🤖 Prompt for AI Agents
In pyproject.toml around line 9, the review missed that upgrading copier to
9.11.0 drops Python 3.9 support; before changing copier, verify and reconcile
the project's Python requirement: check and update the project’s
"requires-python" / classifiers and CI matrix to >=3.10 if you want copier
9.11.0, or else keep/lock copier to the last 3.9-compatible release; pytest
9.0.1 is safe to update. Ensure CI, docs, and any runtime environments are
aligned with the chosen Python baseline before merging.
Pull in upstream template changes, and add timeouts to this template CI
Summary by CodeRabbit
Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.