-
-
Notifications
You must be signed in to change notification settings - Fork 28
Added installer artifact to dev docs Netlify deployment. #2146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe CI now uploads the built PHAR from the "Vortex - Test installer" workflow and the "Vortex - Test docs" workflow is triggered by that workflow's successful completion; the docs workflow downloads the installer artifact into Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant InstallerWF as "Vortex - Test installer\n(GitHub Actions)"
participant ArtifactStore as "GitHub Actions Artifacts"
participant DocsWF as "Vortex - Test docs\n(GitHub Actions)"
InstallerWF->>ArtifactStore: upload artifact\nname: vortex-installer (installer.phar)
note right of ArtifactStore: Artifact stored and indexed
InstallerWF-->>DocsWF: emits workflow_run (on success)
DocsWF->>ArtifactStore: download artifact\nvortex-installer -> .vortex/docs/static/install
DocsWF->>DocsWF: run installer.phar\n(print version)
DocsWF->>DocsWF: continue docs checks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/vortex-test-docs.yml(2 hunks).github/workflows/vortex-test-installer.yml(1 hunks)
⏰ 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). (10)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
🔇 Additional comments (4)
.github/workflows/vortex-test-docs.yml (3)
5-8: Workflow trigger correctly configured for artifact pipeline.The workflow_run trigger is properly set to listen for the "Vortex - Test installer" workflow completion, which enables the downstream docs build to consume the installer artifact.
32-39: Artifact download configuration is correct.The download step properly references the installer workflow and artifact name. The
if_no_artifact_found: failandallow_forks: trueflags are appropriate for this use case.Confirm that the workflow name
vortex-test-installer.ymland artifact namevortex-installermatch the upload step in the corresponding installer workflow file.
41-44: Installer file handling and smoke test look good.The file move preserves the artifact location and the
--versionexecution is a solid smoke test to verify the PHAR binary is functional before proceeding to docs build steps..github/workflows/vortex-test-installer.yml (1)
94-100: Installer artifact upload is correctly configured.The step properly:
- Restricts upload to PHP 8.3 only (appropriate for a production deployment artifact)
- Uses the correct artifact name (
vortex-installer) that downstream workflows expect- Fails fast if the PHAR build doesn't produce the expected file
- Matches the action version used elsewhere in the workflow
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2146 +/- ##
===========================================
- Coverage 76.35% 75.68% -0.67%
===========================================
Files 109 102 -7
Lines 5675 5516 -159
Branches 44 0 -44
===========================================
- Hits 4333 4175 -158
+ Misses 1342 1341 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Upload installer PHAR as artifact in vortex-test-installer (PHP 8.3 only). - Changed vortex-test-docs to trigger via workflow_run after installer completes. - Download and include installer in Netlify docs deployment.
3f02de0 to
cf27c8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/vortex-test-docs.yml(2 hunks).github/workflows/vortex-test-installer.yml(1 hunks)
⏰ 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). (10)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
🔇 Additional comments (3)
.github/workflows/vortex-test-installer.yml (2)
89-92: Test PHAR step enhanced with version output and additional test options.The addition of
--versionoutput and test parameters (--no-interaction,--no-cleanup,destination=test) provides better validation before uploading the artifact. The explicitexit 1on failure ensures the workflow fails if PHAR execution fails.
94-100: Artifact upload correctly conditioned on PHP 8.3 only.The step appropriately uploads only for PHP 8.3 to avoid redundant artifacts, ensuring a single installer artifact is available for the docs workflow to consume. Artifact naming (
vortex-installer) aligns with the download step in vortex-test-docs.yml. Error handling viaif-no-files-found: erroris appropriate..github/workflows/vortex-test-docs.yml (1)
4-14: Correctly simplified trigger condition.The workflow_run trigger and success guard are correctly implemented. The past feedback to remove the dead-code branch has been properly resolved—the condition now checks only
github.event.workflow_run.conclusion == 'success'without extraneous logic.
| - name: Copy installer to docs | ||
| run: | | ||
| mv .vortex/docs/static/installer.phar .vortex/docs/static/install | ||
| php .vortex/docs/static/install --version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Installer preparation logic is sound.
The step appropriately moves the downloaded artifact and verifies it via --version before proceeding. The removal of the .phar extension is acceptable since the file is explicitly invoked via php. Any failure in the move or execution will correctly halt the workflow.
Consider adding a brief comment if the install filename (without extension) is intentional for documentation purposes, to clarify intent for future maintainers.
🤖 Prompt for AI Agents
.github/workflows/vortex-test-docs.yml around lines 41 to 44: add a brief inline
comment above the "Copy installer to docs" step explaining that the .phar
artifact is intentionally renamed to "install" (no extension) because it will be
invoked via "php install" for documentation/testing purposes, so future
maintainers understand the filename change and that the file is executed with
PHP rather than relying on a .phar extension.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.