Repository navigation
ci: allow pnpm builds through env instead of a per-install flag - #305
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Workflow install configuration .github/workflows/benchmark.yml, .github/workflows/main.yml, .github/workflows/pull_request.yml |
The workflows set pnpm’s build allowance through environment variables. Their install commands no longer pass the build allowance flag. The benchmark workflow also sets an environment variable to skip simple Git hook installation. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Refactor
Possibly related PRs
- microlinkhq/html-get#302: Adds dependency installation to the benchmark workflow that this change updates.
- microlinkhq/html-get#304: Adds base and head installation and benchmarking to the workflow that this change updates.
Merge Risk: 
Minimal · up to c572f
-
The workflows preserve pnpm’s build allowance without a merge-blocking behavior identified.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
html-get
Details
| Benchmark suite | Current: c572f21 | Previous: 669d6ba | Ratio |
|---|---|---|---|
html rewriteUrls off |
182.6 ms (176.6-185) |
175.8 ms (174.4-180.7) |
1.04 |
html rewriteUrls on |
266.1 ms (251.3-277.9) |
267.1 ms (250.3-269.2) |
1.00 |
pdf extract |
160 ms (153-162.2) |
157.9 ms (154.4-158.9) |
1.01 |
cold require |
350.2 ms (348-355.3) |
351.6 ms (340.2-358.9) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Why
Each workflow allowed dependency build scripts by passing
--dangerously-allow-all-buildsto everypnpm install. That flag was repeated on each install line, andpnpm/action-setup'srun_installcan't pass it at all. That's how whoops' release broke when pnpmlatestmoved to v12.15.0 (ERR_PNPM_IGNORED_BUILDS).What changed
Every
pnpm install --dangerously-allow-all-buildsis now a plainpnpm install.Each workflow that installed with pnpm sets these once in its top-level
env::An existing top-level
env:keeps its keys, and only the missing ones are added.How it was tested
pnpm installfails on both. Withpnpm_config_dangerously_allow_all_builds=truein the environment it exits 0, andspawn-sync's build script runs.benchmark.ymlits output matches the benchmark workflow generator exactly.actionlintreports the same findings before and after for each changed file, so this change adds none.🤖 Generated with Claude Code
Note
Low Risk
CI-only configuration change with equivalent install behavior; no application or runtime code is touched.
Overview
CI workflows no longer pass
--dangerously-allow-all-buildson eachpnpm install. Instead,benchmark.yml,main.yml, andpull_request.ymlsetpnpm_config_dangerously_allow_all_builds: trueonce in top-levelenv:(benchmark also addsSKIP_INSTALL_SIMPLE_GIT_HOOKSthere). Install steps are plainpnpm install, matching pnpm 12.15+ behavior when build scripts would otherwise be skipped (ERR_PNPM_IGNORED_BUILDS).Reviewed by Cursor Bugbot for commit c572f21. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit