Slim packaged artifacts - #42
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR introduces PHPDoc stripping during PHAR packaging to reduce binary artifact size. A new ChangesPHPDoc Stripping Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 reduces packaged PHAR/static artifact size by stripping non-runtime PHPDoc from packaged PHP files (while preserving reflection-needed docs for cebe/markdown), excluding additional non-runtime vendor/runtime inputs, and moving benchmark fixture generation helpers to dev-only autoload with dependency cleanups.
Changes:
- Add
PhpDocStripperand integrate it intobuild/package-phar.phpso packaged PHP sources have PHPDoc removed (exceptvendor/cebe/markdown). - Tighten packaging filters/scripts to omit additional non-runtime inputs (e.g.
vendor/composer/installed.json,.hhistubs,config/.gitignore) and ensure platform/Docker packaging scripts include the new support file. - Move benchmark dataset generator to
autoload-devand adjust Composer dependencies accordingly.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Packaging/ConfigurationPackagingTest.php | Adds assertions covering PHPDoc stripping behavior, packaging inputs, and Composer autoload expectations. |
| tests/Unit/Benchmarks/BenchmarkFixtureTest.php | Verifies benchmark generator is dev-autoloaded (not part of runtime autoload). |
| docs/binaries-phar-docker.md | Updates documentation to describe stripping and omitted non-runtime inputs. |
| docker/Dockerfile | Ensures PhpDocStripper.php is copied into the PHAR build stage. |
| composer.lock | Updates lockfile to reflect dependency graph changes. |
| composer.json | Adds react/stream, adds evenement/evenement (dev), and configures autoload-dev classmap/PSR-4 for build + benchmark helpers. |
| build/PhpDocStripper.php | Introduces PHPDoc stripping implementation. |
| build/PharArchiveFilter.php | Expands exclusion rules to omit additional non-runtime packaged files. |
| build/package-windows.ps1 | Copies PhpDocStripper.php into Windows packaging workspace. |
| build/package-phar.php | Integrates PhpDocStripper into PHAR assembly process. |
| build/package-macos.sh | Copies PhpDocStripper.php into macOS packaging workspace. |
| benchmarks/src/BenchmarkDataGenerator.php | Adds benchmark fixture dataset generator (intended for dev/test usage). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6b5341d to
e1a6bb4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/binaries-phar-docker.md (1)
31-31: ⚡ Quick winConsider breaking up the long sentence for readability.
Line 31 packs multiple concepts (build inputs, dependency installation, PHPDoc stripping rationale, and exclusions) into a single ~500-character sentence. While technically correct, splitting it into 3-4 shorter sentences would improve readability and make each concept easier to scan.
📝 Suggested refactor for clarity
-The PHAR builder copies only runtime inputs into the build stage: `config/`, `public/`, `src/`, `themes/`, `yii`, Composer metadata, and the PHAR build scripts. Dependencies are installed with `--no-dev` inside that stage before the PHAR is assembled. PHPDoc comments are stripped from packaged PHP files to keep the standalone PHAR and embedded static-binary PHAR smaller while preserving runtime comments, code, and dependency PHPDoc that is read through reflection at runtime. Benchmark fixture helpers, Composer's `installed.json`, VCS placeholders, and non-runtime type stubs are omitted because packaged commands do not need them. +The PHAR builder copies only runtime inputs into the build stage: `config/`, `public/`, `src/`, `themes/`, `yii`, Composer metadata, and the PHAR build scripts. Dependencies are installed with `--no-dev` inside that stage before the PHAR is assembled. + +PHPDoc comments are stripped from packaged PHP files to reduce artifact size while preserving runtime comments, code, and dependency PHPDoc that is read through reflection at runtime. Benchmark fixture helpers, Composer's `installed.json`, VCS placeholders, and non-runtime type stubs are omitted because packaged commands do not need them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/binaries-phar-docker.md` at line 31, Split the long sentence that starts with "The PHAR builder copies only runtime inputs" into 3–4 shorter sentences: one stating which inputs are copied (e.g., `config/`, `public/`, `src/`, `themes/`, `yii`, Composer metadata, PHAR build scripts), a second describing that dependencies are installed with `--no-dev` inside the build stage before assembling the PHAR, a third explaining that PHPDoc comments are stripped to reduce PHAR size while preserving runtime/reflection-read comments and dependency PHPDoc, and a final short sentence listing omitted items (benchmark fixtures, Composer's `installed.json`, VCS placeholders, non-runtime type stubs). Keep wording concise and preserve technical details and rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/binaries-phar-docker.md`:
- Line 31: Split the long sentence that starts with "The PHAR builder copies
only runtime inputs" into 3–4 shorter sentences: one stating which inputs are
copied (e.g., `config/`, `public/`, `src/`, `themes/`, `yii`, Composer metadata,
PHAR build scripts), a second describing that dependencies are installed with
`--no-dev` inside the build stage before assembling the PHAR, a third explaining
that PHPDoc comments are stripped to reduce PHAR size while preserving
runtime/reflection-read comments and dependency PHPDoc, and a final short
sentence listing omitted items (benchmark fixtures, Composer's `installed.json`,
VCS placeholders, non-runtime type stubs). Keep wording concise and preserve
technical details and rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4f5b7fb-753b-4a1c-9c7e-bda58853625f
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
benchmarks/src/BenchmarkDataGenerator.phpbuild/PharArchiveFilter.phpbuild/PhpDocStripper.phpbuild/package-macos.shbuild/package-phar.phpbuild/package-windows.ps1composer.jsondocker/Dockerfiledocs/binaries-phar-docker.mdtests/Unit/Benchmarks/BenchmarkFixtureTest.phptests/Unit/Packaging/ConfigurationPackagingTest.php
|
Addressed Copilot discussion r3344866244 in e1a6bb4: package-local paths are normalized to forward slashes before filtering, PHPDoc stripping, and PHAR entry creation. PharArchiveFilter and PhpDocStripper also normalize paths defensively, with Windows-style path coverage in packaging tests. |
e1a6bb4 to
5a9f07d
Compare
|
Rebased on current master and resolved the composer.lock conflict in 5a9f07d. Also addressed the docs readability nit. Local verification passed: make test, git diff --check, make composer-dependency-analyser, make package-phar, and dist/phar/yiipress.phar --help smoke test. GitHub now reports the PR as mergeable; CI jobs are still pending. |
Summary
Verification
Packaged PHAR size on this branch: 2,861,304 bytes.
Summary by CodeRabbit
New Features
Chores