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. WalkthroughBumped multiple tooling and extension versions, switched Ruff VS Code configuration to an absolute workspace path in devcontainer files, updated pre-commit hook revisions and added excludes for test snapshot directories. No runtime logic changes. Changes
Sequence Diagram(s)(omitted — changes are configuration/version updates without new control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a configuration issue with the ruff VS Code extension in devcontainer environments where ${workspaceFolder} was not working correctly, reverting to explicit paths. Additionally, several project dependencies have been updated to their latest versions.
- Fixed ruff configuration path in devcontainer by replacing
${workspaceFolder}with explicit/workspaces/{repo_name}paths - Updated multiple dependencies including pre-commit (4.2.0 → 4.3.0), copier (9.8.0 → 9.9.1), and pulumi (3.189.0 → 3.190.0)
- Added snapshot directories to pre-commit hook exclusions
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| template/.devcontainer/devcontainer.json.jinja-base | Fixed ruff configuration path and updated ruff extension version |
| extensions/context.py | Updated dependency versions for pre-commit, copier, and pulumi |
| .pre-commit-config.yaml | Updated pre-commit hook versions and added snapshot exclusions |
| .devcontainer/install-ci-tooling.py | Updated copier and pre-commit version constants |
| .devcontainer/devcontainer.json | Applied same ruff configuration fix and updated hash |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # Reformatting (should generally come before any file format or other checks, because reformatting can change things) | ||
| - repo: https://github.com/crate-ci/typos | ||
| rev: 392b78fe18a52790c53f42456e46124f77346842 # frozen: v1.34.0 | ||
| rev: 7fb6e0951ad91e4772a2470012fc1ae621016b80 # frozen: v1 |
There was a problem hiding this comment.
The frozen comment shows 'v1' which is inconsistent with other entries that show specific version numbers like 'v1.34.0'. This makes it unclear which exact version is being used.
| rev: 7fb6e0951ad91e4772a2470012fc1ae621016b80 # frozen: v1 | |
| rev: 7fb6e0951ad91e4772a2470012fc1ae621016b80 # frozen: v1.14.0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
extensions/context.py (1)
11-19: Reduce duplication of version literals across filesYou already note duplication with install-ci-tooling.py. Consider centralizing these version pins (e.g., a single versions.yml/toml consumed by both this hook and the installer), or adding a simple check in CI to enforce parity.
If helpful, I can draft a minimal versions.toml loader and a CI check to validate alignment across files.
Also applies to: 20-29
.devcontainer/install-ci-tooling.py (1)
8-12: Avoid hardcoding version pins in multiple placesOptional: load these from a single source (env file, versions.toml, or even reusing extensions/context.py via import) to prevent drift.
Example approach (env fallback):
COPIER_VERSION = os.getenv("COPIER_VERSION", "9.9.1") PRE_COMMIT_VERSION = os.getenv("PRE_COMMIT_VERSION", "4.3.0")I can also sketch a tiny versions.toml and loader if you want it standardized across Python and Jinja templates.
template/.devcontainer/devcontainer.json.jinja-base (1)
65-65: Absoluteruff.configurationrequires an absolute path – document the rationaleVS Code’s
${workspaceFolder}isn’t expanded by the Ruff extension forruff.configuration(only environment variables and~are supported), so an absolute path is necessary. Add an inline note to explain this and prevent accidental reversion:- "ruff.configuration": "/workspaces/{{ repo_name }}/ruff-test.toml", // use the test configuration since it's less restrictive and won't show false positives and underline things + // NOTE: ${workspaceFolder} isn’t expanded here; `ruff.configuration` only supports env vars/`~`, so we use an absolute path. + "ruff.configuration": "/workspaces/{{ repo_name }}/ruff-test.toml", // use the test configuration since it's less restrictive and won't show false positives and underline thingsOptionally, for portability you could leverage environment‐variable expansion (e.g.
"$WORKSPACE_ROOT/ruff-test.toml").Also confirm that
repo_namealways matches the folder name inside the container for all template consumers..pre-commit-config.yaml (1)
1-4: Minor consistency check: minimum_pre_commit_version vs installerminimum_pre_commit_version is 4.0.1 while the installer pins pre-commit to 4.3.0. This is fine, but keep in mind the floor doesn’t enforce the exact version. If you intend to guarantee 4.3.0 features, consider aligning the minimum accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.devcontainer/devcontainer.json(3 hunks).devcontainer/install-ci-tooling.py(1 hunks).pre-commit-config.yaml(8 hunks)extensions/context.py(2 hunks)template/.devcontainer/devcontainer.json.jinja-base(2 hunks)
🔇 Additional comments (13)
extensions/context.py (3)
15-17: Version bumps aligned; good updatepre_commit_version -> 4.3.0 and copier_version -> 9.9.1 look consistent with the rest of the PR.
28-28: Pulumi minor upgrade looks fine3.190.0 is a routine bump with no code flow changes here.
15-17: Cross-file versions are alignedVerified that
pre_commit_version(4.3.0) andcopier_version(9.9.1) inextensions/context.pyexactly matchPRE_COMMIT_VERSIONandCOPIER_VERSIONin.devcontainer/install-ci-tooling.py. No further updates required..devcontainer/install-ci-tooling.py (1)
9-11: Tooling versions bumped appropriatelyCOPIER_VERSION -> 9.9.1 and PRE_COMMIT_VERSION -> 4.3.0 align with extensions/context.py updates.
template/.devcontainer/devcontainer.json.jinja-base (1)
38-38: Ruff VS Code extension version bump is fine2025.24.0 looks consistent with the root devcontainer.
.devcontainer/devcontainer.json (3)
32-32: Ruff extension bump aligns with the templateUpgrading to 2025.24.0 is consistent and low risk.
62-62: Context hash updatedPre-commit managed hash looks updated. No action needed.
51-51: Absolute ruff.configuration path verifiedThe
ruff-test.tomlfile is present at the repository root, so the hard-coded path in.devcontainer/devcontainer.jsonwill resolve correctly in the container.No changes needed.
.pre-commit-config.yaml (5)
45-45: typos hook pinned and updatedMoving to a newer commit (frozen) is fine; keeping it pinned is good practice.
56-56: pre-commit-hooks bumped to v6.0.0 across sectionsConsistent upgrades across sections; looks good.
Also applies to: 157-158, 207-208
63-64: Excluding test snapshots from formatters is sensibleAvoids churn and noise from generated snapshots.
Also applies to: 76-77, 91-92
229-229: Ruff pre-commit hook updated to v0.12.9Matches the general tooling refresh; no concerns.
242-242: Pylint bumped to v3.3.8Straightforward version bump; should be fine.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.pre-commit-config.yaml (1)
45-45: Clarify frozen version for typos: replace ambiguous "v1" with the exact tag.This mirrors an existing review: “frozen: v1” is ambiguous compared to explicit tags elsewhere. Pin the comment to the precise version for clarity.
- rev: 7fb6e0951ad91e4772a2470012fc1ae621016b80 # frozen: v1 + rev: 7fb6e0951ad91e4772a2470012fc1ae621016b80 # frozen: v1.14.0
🧹 Nitpick comments (4)
.pre-commit-config.yaml (4)
1-1: Align minimum_pre_commit_version with the rest of the repo (4.3.0).Other files in this PR bump pre-commit to 4.3.0; consider aligning the minimum here for consistency and to avoid confusion.
-minimum_pre_commit_version: 4.2.0 +minimum_pre_commit_version: 4.3.0
58-65: Snapshot exclude pattern may miss top-level tests/snapshots.The pattern
.*tests/.*/__snapshots__/.*requires at least one directory betweentests/and__snapshots__. It won’t matchtests/__snapshots__/.... Use a simpler and more robust pattern to catch snapshots anywhere.- .*tests/.*/__snapshots__/.*| + .*__snapshots__/.*|
66-81: Extend end-of-file-fixer exclude: same snapshot pattern issue.Apply the same robust pattern so top-level snapshot dirs are excluded here as well.
- .*tests/.*/__snapshots__/.*| + .*__snapshots__/.*|
82-93: Extend pretty-format-json exclude: same snapshot pattern issue.Unify on the simpler
.*__snapshots__/.*so all snapshot directories are consistently excluded.- .*tests/.*/__snapshots__/.*| + .*__snapshots__/.*|
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.devcontainer/devcontainer.json(3 hunks).pre-commit-config.yaml(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/devcontainer.json
⏰ 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). (2)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.12.7, --data-file tests/copier_data/data1.yaml)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.13.1, --data-file tests/copier_data/data1.yaml)
🔇 Additional comments (3)
.pre-commit-config.yaml (3)
56-56: LGTM on pre-commit-hooks bumps to v6.0.0.Pinned to commit SHAs with clear frozen tags; no concerns.
Also applies to: 157-157, 207-207
242-249: pylintrc.toml presence confirmed
Thepylintrc.tomlfile was found at the repository root, so the hook’s--rcfilereference is valid.
229-239: Ruff configs exist and pre-commit entries are correct.Both ruff.toml and ruff-test.toml are present at the repo root, and the
.pre-commit-config.yamlhooks correctly reference--config=./ruff.toml(lines 233–234) and--config=./ruff-test.toml(lines 237–238). No further changes needed.
Why is this change necessary?
Copilot lied about using workspaceFolder, it doesn't work
How does this change address the issue?
reverts to explicit path
What side effects does this change have?
Things work again
How is this change tested?
They worked before
Other
Bumped some dependencies
Ignoring snapshots for more pre-commit hooks
Summary by CodeRabbit
New Features
Bug Fixes
Chores