Conversation
|
Warning Rate limit exceeded@ejfine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 34 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (4)
WalkthroughBumps template and tooling versions, updates dependency/version context, adjusts pre-commit and Pyright exclusions, and introduces a new --only-create-lock mode in manual-setup-deps scripts (root and template). Minor README and Devcontainer comment updates accompany these changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Script as manual-setup-deps.py
participant UV as uv
participant PNPM as pnpm
Dev->>Script: Run with flags
alt --only-create-lock
Note over Script: Generate lock files only
Script->>UV: uv lock --directory <env> (no --check)
Script--x PNPM: Skip install/sync
Script--x UV: Skip uv sync
else Install/Check flow
Note over Script: Check lock and install
opt env_check_lock
Script->>UV: uv lock --directory <env> --check
end
alt env_check_lock
Script->>UV: uv sync --frozen
Script->>PNPM: pnpm install --frozen-lockfile
else
Script->>UV: uv sync
Script->>PNPM: pnpm install
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 2 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.
Pull Request Overview
This PR updates the Copier template by bumping various dependency versions, particularly pytest-cov from 6.3.0 to 7.0.0, and adds support for excluding generated code directories (graphql_codegen and openapi_codegen) from linting and type checking tools.
- Updates pytest-cov, pytest-randomly, copier, and other dependencies to newer versions
- Adds exclusions for generated code directories across multiple linting tools (ruff, pylint, pyright)
- Enhances the manual setup script with a new
--only-create-lockoption for lock file generation
Reviewed Changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| template/pyrightconfig.json | Adds exclusions for generated GraphQL and OpenAPI code directories |
| template/.pre-commit-config.yaml | Updates typos and ruff versions, adds exclusions for generated code directories |
| template/.devcontainer/manual-setup-deps.py | Adds --only-create-lock flag and refactors lock file checking logic |
| pyproject.toml | Bumps pytest-cov, pytest-randomly, and copier dependency versions |
| extensions/context.py | Updates version numbers for multiple dependencies and tools |
| README.md | Updates setup instructions to use the new --only-create-lock flag |
| .devcontainer/install-ci-tooling.py | Bumps UV and Copier versions |
| .devcontainer/devcontainer.json | Updates devcontainer context hash |
| .copier-answers.yml | Updates commit reference |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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 (2)
.devcontainer/manual-setup-deps.py (1)
108-126: Bug: --only-create-lock still installs PNPM depsIn "lock-only" mode we still run pnpm install, which contradicts the flag semantics and README. Gate PNPM install behind generate_lock_file_only, and generate the lock without installing.
Apply:
- elif env.package_manager == PackageManager.PNPM: - pnpm_command = ["pnpm", "install", "--dir", str(env.path)] - if env_check_lock: - pnpm_command.append("--frozen-lockfile") - if is_windows: - pwsh = shutil.which("pwsh") or shutil.which("powershell") - if not pwsh: - raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") - pnpm_command = [ - pwsh, - "-NoProfile", - "-NonInteractive", - "-Command", - " ".join(pnpm_command), - ] - _ = subprocess.run( - pnpm_command, - check=True, - ) + elif env.package_manager == PackageManager.PNPM: + if generate_lock_file_only: + # Create/update lockfile without installing + _ = subprocess.run( + ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"], + check=True, + ) + else: + pnpm_command = ["pnpm", "install", "--dir", str(env.path)] + if env_check_lock: + pnpm_command.append("--frozen-lockfile") + if is_windows: + pwsh = shutil.which("pwsh") or shutil.which("powershell") + if not pwsh: + raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") + # Quote the path for spaces on Windows PowerShell + pnpm_command = [ + pwsh, + "-NoProfile", + "-NonInteractive", + "-Command", + f'& pnpm install --dir "{str(env.path)}"' + (" --frozen-lockfile" if env_check_lock else ""), + ] + _ = subprocess.run(pnpm_command, check=True)template/.devcontainer/manual-setup-deps.py (1)
108-126: Mirror fix: prevent PNPM install in lock-only mode and generate lockfileSame issue as the non-template script. Apply the same gating and lockfile-only behavior.
- elif env.package_manager == PackageManager.PNPM: - pnpm_command = ["pnpm", "install", "--dir", str(env.path)] - if env_check_lock: - pnpm_command.append("--frozen-lockfile") - if is_windows: - pwsh = shutil.which("pwsh") or shutil.which("powershell") - if not pwsh: - raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") - pnpm_command = [ - pwsh, - "-NoProfile", - "-NonInteractive", - "-Command", - " ".join(pnpm_command), - ] - _ = subprocess.run( - pnpm_command, - check=True, - ) + elif env.package_manager == PackageManager.PNPM: + if generate_lock_file_only: + _ = subprocess.run( + ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"], + check=True, + ) + else: + pnpm_command = ["pnpm", "install", "--dir", str(env.path)] + if env_check_lock: + pnpm_command.append("--frozen-lockfile") + if is_windows: + pwsh = shutil.which("pwsh") or shutil.which("powershell") + if not pwsh: + raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") + pnpm_command = [ + pwsh, + "-NoProfile", + "-NonInteractive", + "-Command", + f'& pnpm install --dir "{str(env.path)}"' + (" --frozen-lockfile" if env_check_lock else ""), + ] + _ = subprocess.run(pnpm_command, check=True)
🧹 Nitpick comments (10)
.devcontainer/install-ci-tooling.py (2)
67-72: Harden curl installer calls with timeouts/retries.Reduce flaky network failures and hangs during CI.
- _ = subprocess.run( - f"curl -fsSL https://astral.sh/uv/{UV_VERSION}/install.sh | sh", + _ = subprocess.run( + f"curl -fsSL --connect-timeout 20 --max-time 120 --retry 3 --retry-delay 2 https://astral.sh/uv/{UV_VERSION}/install.sh | sh", check=True, shell=True, env=uv_env, )Also consider adding the same flags to other curl downloads (Lines 128–132 and 145–149).
128-132: Apply curl resiliency flags to artifact downloads.Same rationale as above.
- "curl", - "https://s3.amazonaws.com/session-manager-downloads/plugin/1.2.707.0/windows/SessionManagerPluginSetup.exe", + "curl", "--fail", "--silent", "--show-error", + "--connect-timeout", "20", "--max-time", "120", + "--retry", "3", "--retry-delay", "2", + "https://s3.amazonaws.com/session-manager-downloads/plugin/1.2.707.0/windows/SessionManagerPluginSetup.exe", "-o", f"{local_package_path}",- "curl", - "https://s3.amazonaws.com/session-manager-downloads/plugin/1.2.707.0/ubuntu_64bit/session-manager-plugin.deb", + "curl", "--fail", "--silent", "--show-error", + "--connect-timeout", "20", "--max-time", "120", + "--retry", "3", "--retry-delay", "2", + "https://s3.amazonaws.com/session-manager-downloads/plugin/1.2.707.0/ubuntu_64bit/session-manager-plugin.deb", "-o", f"{local_package_path}",Also applies to: 145-149
.pre-commit-config.yaml (2)
26-28: Comment/code drift in uv sync hook.The comment mentions checking for the phrase “operations”, but the code checks for “Installed”. Align the comment or the check to reduce confusion.
- # When the install does change things, it exits 0, so needed to check for the phrase 'operations' in stdout in order to cause things pre-commit command to fail - entry: python -c "import subprocess,sys; results=subprocess.run(['uv', 'sync', '--frozen'],timeout=360,capture_output=True); print ('stdout- ' + results.stdout.decode('utf-8', 'strict')); print ('stderr- ' + results.stderr.decode('utf-8', 'strict')); sys.exit(int(results.returncode or int('Installed' in results.stdout.decode('utf-8', 'strict'))));" + # uv sync exits 0 on no-ops; detect actual installs via 'Installed' in stdout to fail the hook and force a commit with the lock updates + entry: python -c "import subprocess,sys; results=subprocess.run(['uv', 'sync', '--frozen'],timeout=360,capture_output=True); print('stdout- ' + results.stdout.decode()); print('stderr- ' + results.stderr.decode()); sys.exit(int(results.returncode or int('Installed' in results.stdout.decode())))"
105-129: Prettier mirror is archived — plan a migration path.Consider switching to Biome-only for formatting or invoking prettier via npm (e.g., local repo with package.json + pre-commit’s language: node) to avoid relying on an archived mirror.
.devcontainer/manual-setup-deps.py (3)
112-122: Windows quoting risk for paths with spacesJoining the command into a single PowerShell string can break when env.path contains spaces. The diff above switches to an explicit PowerShell command string that quotes the directory. Alternatively, avoid PowerShell and invoke pnpm directly via subprocess on Windows.
64-66: Flag interplay is sound; consider tiny readability tweakLogic is correct, but the double-negative makes it harder to scan. Rename/check expression for clarity.
- generate_lock_file_only = args.only_create_lock - check_lock_file = not (args.skip_check_lock or args.optionally_check_lock or generate_lock_file_only) + generate_lock_file_only = args.only_create_lock + should_check_lock = not (args.skip_check_lock or args.optionally_check_lock or generate_lock_file_only) ... - env_check_lock = check_lock_file + env_check_lock = should_check_lockAlso applies to: 81-85, 100-107
93-94: Subprocess hardening (timeout, logging)Add timeouts to avoid hanging in CI/devcontainers and include command echo for easier diagnostics.
- _ = subprocess.run(uv_args, check=True, env=uv_env) + _ = subprocess.run(uv_args, check=True, env=uv_env, timeout=600) ... - _ = subprocess.run( + _ = subprocess.run( sync_command, - check=True, - env=uv_env, + check=True, + env=uv_env, + timeout=900, ) ... - _ = subprocess.run( - pnpm_command, - check=True, - ) + _ = subprocess.run(pnpm_command, check=True, timeout=900)Also applies to: 103-107, 123-126
template/.devcontainer/manual-setup-deps.py (2)
112-122: Quote Windows path or avoid PowerShell wrapperEnsure paths with spaces are quoted (see diff above), or prefer invoking pnpm directly without PowerShell.
93-94: Add timeouts to subprocess callsPrevents hangs in CI/Dev Containers.
- _ = subprocess.run(uv_args, check=True, env=uv_env) + _ = subprocess.run(uv_args, check=True, env=uv_env, timeout=600) ... - _ = subprocess.run( + _ = subprocess.run( sync_command, check=True, - env=uv_env, + env=uv_env, + timeout=900, ) ... - _ = subprocess.run( - pnpm_command, - check=True, - ) + _ = subprocess.run(pnpm_command, check=True, timeout=900)Also applies to: 103-107, 123-126
template/.pre-commit-config.yaml (1)
105-129: Archived mirrors-prettier hookThe mirrors-prettier repo is archived per your TODO. Consider moving to an actively maintained wrapper (e.g., pre-commit’s “prettier” hook via native npm/pnpm or a maintained mirror) when convenient.
Would you like me to propose a replacement config that uses pnpm-managed prettier so it respects your lockfile?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.copier-answers.yml(1 hunks).devcontainer/devcontainer.json(1 hunks).devcontainer/install-ci-tooling.py(1 hunks).devcontainer/manual-setup-deps.py(3 hunks).pre-commit-config.yaml(3 hunks)README.md(1 hunks)extensions/context.py(2 hunks)pyproject.toml(1 hunks)pyrightconfig.json(1 hunks)template/.devcontainer/manual-setup-deps.py(3 hunks)template/.pre-commit-config.yaml(3 hunks)template/pyrightconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
template/.devcontainer/manual-setup-deps.py (1)
.devcontainer/manual-setup-deps.py (1)
PackageManager(39-41)
.devcontainer/manual-setup-deps.py (1)
template/.devcontainer/manual-setup-deps.py (1)
PackageManager(39-41)
🪛 Ruff (0.12.2)
template/.devcontainer/manual-setup-deps.py
93-93: subprocess call: check for execution of untrusted input
(S603)
103-103: subprocess call: check for execution of untrusted input
(S603)
.devcontainer/manual-setup-deps.py
93-93: subprocess call: check for execution of untrusted input
(S603)
103-103: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (16)
template/pyrightconfig.json (1)
13-14: LGTM: mirrored excludes in template.Keeps template consumers aligned with repo defaults.
.devcontainer/devcontainer.json (1)
65-65: Devcontainer context hash updated.No functional changes. Good.
.devcontainer/install-ci-tooling.py (2)
10-10: UV pinned to 0.8.17 — OK.Matches context and tooling updates.
12-12: Copier pinned to 9.10.2 — OK.Consistent with pyproject and context.
.copier-answers.yml (2)
2-2: Template commit pointer updated — OK.No action needed.
1-17: Verify Issue #58 — publish workflows are gated for compiled executables (resolved).
template/.github/workflows/{% if not is_frozen_executable %}publish.yaml{% endif %}.jinja and template/.github/workflows/{% if not is_frozen_executable %}publish_to_staging.yaml{% endif %}.jinja are wrapped with{% if not is_frozen_executable %} ... {% endif %}, so PyPI publish workflows are excluded when producing a frozen executable..pre-commit-config.yaml (6)
45-53: LGTM: typos exclude for minified artifacts.Avoids false positives on bundled assets.
65-78: EOF fixer excludes clarified.Comment explains rationale; pattern looks fine.
226-236: Ruff bump and codegen excludes — good.Prevents linting churn on generated code.
241-251: Ruff tests/format excludes — good.
258-265: Pylint excludes template and codegen — good.
275-287: Pyright local hook: exclude/codegen + serial execution — good.Nice to keep it single-run with require_serial.
extensions/context.py (2)
13-21: Python toolchain bumps match pyproject — good.uv/pytest* and copier versions are in sync with pyproject.
83-84: Approve: use windows-2025 as GitHub-hosted runnerGitHub officially supports "windows-2025"; no change required for extensions/context.py (lines 83–84).
pyrightconfig.json (1)
13-14: Approve: exclude generated graphql/openapi codegen from Pyright & pre-commitBoth root and template pyrightconfig.json contain
**/graphql_codegenand**/openapi_codegen(lines 13–14); root and template.pre-commit-config.yamlcontain matching exclude regexes (.*/graphql_codegen/.*,.*/openapi_codegen/.*). No further changes needed.template/.pre-commit-config.yaml (1)
45-54: Pre-commit updates look good; excludes for codegen are helpfulVersions and excludes align with reduced noise from generated sources. No blockers.
Also applies to: 226-252, 256-265, 271-288
| elif env.package_manager == PackageManager.PNPM: | ||
| pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202 | ||
| else: | ||
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") |
There was a problem hiding this comment.
Support PNPM lock generation (no-op today)
When env_check_lock or lock-only is requested, PNPM path is a pass. Add a lockfile-only call here when generate_lock_file_only is true so pnpm projects also get locks created.
- elif env.package_manager == PackageManager.PNPM:
- pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202
+ elif env.package_manager == PackageManager.PNPM:
+ if generate_lock_file_only:
+ _ = subprocess.run(
+ ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"],
+ check=True,
+ )
+ # No strict "check" mode in pnpm; we validate via a subsequent frozen install.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif env.package_manager == PackageManager.PNPM: | |
| pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202 | |
| else: | |
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") | |
| elif env.package_manager == PackageManager.PNPM: | |
| if generate_lock_file_only: | |
| _ = subprocess.run( | |
| ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"], | |
| check=True, | |
| ) | |
| # No strict "check" mode in pnpm; we validate via a subsequent frozen install. | |
| else: | |
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") |
🤖 Prompt for AI Agents
In .devcontainer/manual-setup-deps.py around lines 94 to 97, the PNPM branch
currently has a noop; when generate_lock_file_only (or the equivalent flag used
for env_check_lock/lock-only) is requested we should run PNPM's lock-only
install so a lockfile gets created. Replace the pass with logic that, if
generate_lock_file_only is true, invokes pnpm to generate a lock (e.g. run "pnpm
install --lockfile-only" or equivalent via the existing subprocess/run helper),
propagate or log errors on failure, and otherwise keep it as a no-op for normal
checks.
| context["graphql_codegen_cli_version"] = "^6.0.0" | ||
| context["graphql_codegen_typescript_version"] = "^5.0.0" | ||
| context["graphql_codegen_typescript_operations_version"] = "^5.0.0" | ||
| context["tailwindcss_version"] = "^4.1.11" | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New codegen/tailwind entries — ensure pre-commit excludes are comprehensive.
Looks covered in this PR; just double-check template-side excludes too.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'graphql_codegen|openapi_codegen|tailwind' template/.pre-commit-config.yaml template/pyrightconfig.jsonLength of output: 853
Add tailwind exclude entries to template configs
graphql/openapi codegen excludes exist (template/pyrightconfig.json — lines 13–14; template/.pre-commit-config.yaml — entries around 234–278). Tailwind is not excluded — add matching patterns:
- template/pyrightconfig.json: add "**/tailwindcss" to the exclude array.
- template/.pre-commit-config.yaml: add "./tailwindcss/.|" into the same regex block that contains ./graphql_codegen/. and ./openapi_codegen/..
🤖 Prompt for AI Agents
In extensions/context.py around lines 61–65, the project adds codegen/version
context but doesn't propagate Tailwind to template exclude lists; update
template/pyrightconfig.json by adding "**/tailwindcss" to the "exclude" array
(same area as the existing graphql/openapi excludes) and update
template/.pre-commit-config.yaml by inserting ".*/tailwindcss/.*|" into the same
regex block that currently contains ".*/graphql_codegen/.*" and
".*/openapi_codegen/.*" so Tailwind-generated files are excluded by both pyright
and pre-commit checks.
| "pytest-cov>=7.0.0", | ||
| "pytest-randomly>=4.0.1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Version bumps look good; verify alignment and compatibility.
pytest-cov>=7.0.0, pytest-randomly>=4.0.1, copier>=9.10.2 align with context updates. Please verify:
- Compatibility with pytest 8.4.2.
- Versions mirrored in extensions/context.py.
- Python version (>=3.12.7) matches devcontainer and Copier answers.
Also applies to: 13-13
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "pyproject deps:"
rg -n 'pytest(\-cov|\-randomly)|copier>=' pyproject.toml
echo "context versions:"
rg -n 'pytest_(cov|randomly)_version|copier_version' extensions/context.py
echo "Python versions across files:"
rg -n 'requires-python|python_version|python_ci_versions|py31[23]_version' pyproject.toml .copier-answers.yml extensions/context.pyLength of output: 575
Fix unsatisfiable pytest plugin version bumps
- pyproject.toml pins pytest-cov>=7.0.0 and pytest-randomly>=4.0.1 (pyproject lines 10–13) and extensions/context.py mirrors them (lines 18–20); Python >=3.12.7 is consistent across pyproject.toml, .copier-answers.yml and extensions/context.py (py312_version).
- PyPI currently shows pytest-cov latest 6.2.1 and pytest-randomly latest 3.x — requiring >=7.0.0 / >=4.0.1 is likely unsatisfiable; revert to published plugin versions or update to released versions that explicitly support pytest 8.4.2. (pypi.org)
- Actions: update pyproject.toml and extensions/context.py to installable versions (or relax the constraints) — locations: pyproject.toml (lines 10–13), extensions/context.py (lines 18–20).
🤖 Prompt for AI Agents
In pyproject.toml (lines 10–11) and extensions/context.py (lines 18–20) the
pytest plugin constraints require non-existent major versions (pytest-cov>=7.0.0
and pytest-randomly>=4.0.1); change those pins to published, installable
versions (e.g., pytest-cov==6.2.1 and pytest-randomly==3.x) or relax to
compatible ranges (e.g., pytest-cov>=6.2.1,<7 and pytest-randomly>=3.0,<4) so
the dependencies are satisfiable with pytest 8.4.2, updating both files
consistently.
| 1. Delete all files currently in the repository. Optional...but makes it easiest to avoid git conflicts. | ||
| 1. Run copier to instantiate the template: `copier copy --trust gh:LabAutomationAndScreening/copier-python-package-template.git .` | ||
| 1. Run `python .devcontainer/manual-setup-deps.py --optionally-check-lock` to generate the lock file(s) | ||
| 1. Run `python .devcontainer/manual-setup-deps.py --only-create-lock` to generate the lock file(s) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Docs mismatch due to PNPM behavior
The step says “--only-create-lock” generates lock files without installing. Current script still installs PNPM deps; fix the script as suggested or clarify PNPM behavior.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "--- .devcontainer listing ---"
ls -la .devcontainer || true
echo
echo "--- Show README.md (first 160 lines) ---"
sed -n '1,160p' README.md || true
echo
echo "--- Show .devcontainer/manual-setup-deps.py (up to 400 lines) ---"
if [ -f .devcontainer/manual-setup-deps.py ]; then
sed -n '1,400p' .devcontainer/manual-setup-deps.py
else
echo "File not found: .devcontainer/manual-setup-deps.py"
fi
echo
echo "--- Grep for related keywords across repo ---"
grep -nHR --color=never -E "--only-create-lock|only-create-lock|pnpm|pnpm install|pnpm i|--lock|--lockfile-only|lockfile-only|--frozen-lockfile|--prefer-offline" . || trueLength of output: 8870
Docs mismatch: --only-create-lock does not prevent pnpm from installing
.manual-setup-deps.py's PNPM branch unconditionally runs pnpm install (the elif env.package_manager == PackageManager.PNPM block in .devcontainer/manual-setup-deps.py), so either: skip running pnpm when --only-create-lock is set, or update README.md (the step that mentions --only-create-lock) to explicitly state PNPM will still install because it lacks a lock-only mode.
🤖 Prompt for AI Agents
In README.md around line 13 and in .devcontainer/manual-setup-deps.py in the
PNPM branch (the elif env.package_manager == PackageManager.PNPM block), fix the
docs-code mismatch by making the script respect the --only-create-lock flag for
PNPM: detect the flag and run pnpm in lockfile-only mode (pnpm install
--lockfile-only) when set, otherwise run the normal pnpm install; alternatively,
if you prefer not to change behavior, update README.md to explicitly state PNPM
will still run a full install because the script lacks a lock-only flow.
| elif env.package_manager == PackageManager.PNPM: | ||
| pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202 | ||
| else: | ||
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") |
There was a problem hiding this comment.
Add PNPM lock generation in the lock/check phase
Match uv behavior by generating pnpm lock with --lockfile-only during "only-create-lock".
- elif env.package_manager == PackageManager.PNPM:
- pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202
+ elif env.package_manager == PackageManager.PNPM:
+ if generate_lock_file_only:
+ _ = subprocess.run(
+ ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"],
+ check=True,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif env.package_manager == PackageManager.PNPM: | |
| pass # doesn't seem to be a way to do this https://github.com/orgs/pnpm/discussions/3202 | |
| else: | |
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") | |
| elif env.package_manager == PackageManager.PNPM: | |
| if generate_lock_file_only: | |
| _ = subprocess.run( | |
| ["pnpm", "install", "--dir", str(env.path), "--lockfile-only"], | |
| check=True, | |
| ) | |
| else: | |
| raise NotImplementedError(f"Package manager {env.package_manager} does not support lock file checking") |
Why is this change necessary?
Pull in upstream changes
How is this change tested?
Downstream repos
Other
Also resolves #58 by conditionally including those workflows
Summary by CodeRabbit
New Features
Documentation
Chores
Tests/Quality