Report exact installed YiiPress version - #133
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR embeds a Git commit SHA into packaged binaries and PHAR archives. ChangesCommit metadata propagation and version resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as CI Workflow
participant Docker as docker/Dockerfile
participant Phar as package-phar.php
participant Binary as Packaged Binary
Workflow->>Docker: build-arg YIIPRESS_COMMIT, COMPOSER_ROOT_VERSION
Docker->>Phar: run with YIIPRESS_COMMIT env
Phar->>Binary: embed COMMIT constant in ApplicationInfo.php
Workflow->>Binary: run --version
Binary-->>Workflow: report version matching release tag or commit SHA
sequenceDiagram
participant User as install.sh
participant Checksums as SHA256SUMS endpoint
participant Release as Release URL
User->>Checksums: request SHA256SUMS for latest
Checksums-->>User: redirect to resolved release URL
User->>User: derive version from resolved URL
User->>Release: download archive from resolved URL
Release-->>User: return packaged binary
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 improves YiiPress version reporting by resolving the exact release tag during installation, and by embedding reproducible version identifiers (release tags for tagged builds; full commit SHAs for development artifacts) into packaged outputs. It also tightens CI smoke tests to verify --version across Linux/macOS/Windows artifacts and documents the behavior.
Changes:
- Resolve the effective GitHub release download URL for
latestininstall.shso the installer prints the exact tag before downloading. - Rework
ApplicationInfo::version()to prefer a stable Composer pretty version, otherwise fall back to an embedded commit SHA or Composer reference/placeholder. - Thread
YIIPRESS_COMMIT/COMPOSER_ROOT_VERSIONthrough Docker build + workflows, and update/extend unit tests to validate the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Packaging/InstallerTest.php | Extends the curl stub and assertions to verify the installer prints the resolved latest tag. |
| tests/Unit/Packaging/ConfigurationPackagingTest.php | Updates workflow expectations to validate --version smoke tests and commit/tag propagation. |
| tests/Unit/ApplicationInfoTest.php | Adjusts version expectations to allow release tags, commit SHAs, or unknown, and asserts default COMMIT behavior in tests. |
| tests/Console/ConsoleRunnerTest.php | Updates console banner expectation to match new default version fallback behavior. |
| src/ApplicationInfo.php | Implements new version resolution rules (stable tag first; otherwise commit/reference/unknown). |
| Makefile | Exports YIIPRESS_COMMIT and passes it into Docker packaging builds. |
| install.sh | Resolves latest to the final release URL and prints the exact version tag before downloading assets. |
| docs/binaries-phar-docker.md | Documents installer/version reporting semantics for releases vs development artifacts. |
| docker/Dockerfile | Adds build args/env wiring so PHAR packaging can embed commit and tagged builds can set root version. |
| build/package-phar.php | Validates YIIPRESS_COMMIT and injects it into ApplicationInfo for packaged artifacts. |
| .github/workflows/release.yml | Passes version/commit build args and changes smoke tests to assert exact --version output. |
| .github/workflows/package-static.yml | Passes commit build arg and updates smoke tests to assert --version output in nightly builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/Unit/Packaging/ConfigurationPackagingTest.php (1)
284-306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for every platform version check.
The new PHPUnit assertions do not cover every changed workflow behavior. The package-static test checks the Linux build argument and only the Windows/macOS
--versioninvocations. It does not check the inheritedYIIPRESS_COMMITvalue or the complete equality checks. The release test also omits theYIIPRESS_COMMITDocker build argument and the complete Windows equality check.Add exact assertions for these strings so a regression cannot remove commit propagation or weaken a smoke test while the PHPUnit test still passes.
Suggested assertions
+ self::assertStringContainsString('YIIPRESS_COMMIT: ${{ github.sha }}', $workflow); + self::assertStringContainsString( + 'if ((./dist/windows-amd64/yiipress.exe --version) -ne "YiiPress $env:GITHUB_SHA")', + $workflow, + ); + self::assertStringContainsString( + 'test "$(./dist/macos-arm64/yiipress --version)" = "YiiPress ${GITHUB_SHA}"', + $workflow, + ); + self::assertStringContainsString('YIIPRESS_COMMIT=${{ github.sha }}', $workflow); + self::assertStringContainsString( + 'if ((./dist/windows-amd64/yiipress.exe --version) -ne "YiiPress $env:GITHUB_REF_NAME")', + $workflow, + );As per coding guidelines,
**/*Test.php: For each piece of code add a test using phpunit.Also applies to: 459-470
🤖 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 `@tests/Unit/Packaging/ConfigurationPackagingTest.php` around lines 284 - 306, Update the workflow assertions in the affected packaging tests, including the corresponding release-test section, to cover every platform version check and exact equality condition. Add exact string assertions for inherited YIIPRESS_COMMIT propagation, the Linux build argument, the complete Windows and macOS version invocations/equality checks, and the release Docker YIIPRESS_COMMIT argument, using the existing PHPUnit assertion style.Source: Coding guidelines
🤖 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.
Inline comments:
In `@install.sh`:
- Around line 62-69: The latest-version branch derives version from curl’s final
URL, which may include a query string and produce an invalid tag. Update the
latest resolution logic in install.sh to resolve the GitHub release tag before
downloading SHA256SUMS or the archive, using redirect Location parsing or the
releases API; ensure release_url and version contain only the canonical release
path/tag without query parameters.
In `@src/ApplicationInfo.php`:
- Around line 19-23: Update the version validation in the ApplicationInfo
version-resolution logic to reject Composer development aliases ending in
“-dev”, including values such as “1.0.x-dev”, before returning $version.
Preserve acceptance of valid release versions and the existing fallback order
through COMMIT, InstalledVersions::getReference('yiipress/engine'), and
self::VERSION.
In `@tests/Console/ConsoleRunnerTest.php`:
- Line 23: Update the assertion in ConsoleRunnerTest to derive the expected
version from ApplicationInfo::version(), or establish deterministic Composer
metadata with an empty COMMIT before invoking the console runner; preserve the
existing output assertion while avoiding a hardcoded “YiiPress unknown” value.
In `@tests/Unit/ApplicationInfoTest.php`:
- Around line 17-18: Update the version assertion in the ApplicationInfo version
test to anchor the regular expression at the end, ensuring the complete value
matches only a semantic version, 40-character commit hash, or “unknown”; add
exact assertions for fallback values where applicable.
In `@tests/Unit/Packaging/InstallerTest.php`:
- Around line 46-59: Update the mock curl handling in the InstallerTest fixture
so the %{url_effective} response appends a representative query string after the
asset filename, such as signed CDN parameters. Extend the installer assertions
to verify it still extracts the correct version and downloads the expected asset
URL when the effective URL has trailing query parameters.
---
Nitpick comments:
In `@tests/Unit/Packaging/ConfigurationPackagingTest.php`:
- Around line 284-306: Update the workflow assertions in the affected packaging
tests, including the corresponding release-test section, to cover every platform
version check and exact equality condition. Add exact string assertions for
inherited YIIPRESS_COMMIT propagation, the Linux build argument, the complete
Windows and macOS version invocations/equality checks, and the release Docker
YIIPRESS_COMMIT argument, using the existing PHPUnit assertion style.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 820429e3-48a4-4996-ae73-4764c0aac457
📒 Files selected for processing (12)
.github/workflows/package-static.yml.github/workflows/release.ymlMakefilebuild/package-phar.phpdocker/Dockerfiledocs/binaries-phar-docker.mdinstall.shsrc/ApplicationInfo.phptests/Console/ConsoleRunnerTest.phptests/Unit/ApplicationInfoTest.phptests/Unit/Packaging/ConfigurationPackagingTest.phptests/Unit/Packaging/InstallerTest.php
Summary
Tests
make test tests/Unit/Packaging/InstallerTest.php tests/Unit/ApplicationInfoTest.php tests/Unit/Packaging/ConfigurationPackagingTest.php— passed, 39 tests and 595 assertionsmake test tests/Unit/Packaging/ConfigurationPackagingTest.php tests/Unit/ApplicationInfoTest.php— passed, 32 tests and 488 assertionsThe full local suite is currently affected by CRLF checkout line endings: executable tests fail with
/usr/bin/env: php\r: No such file or directory.Summary by CodeRabbit
Release Notes
New Features
Documentation