[#2596] Added installer support for multiple Vortex scaffold versions (1.x and 2.x).#2605
Conversation
… '/v2/install' in CI.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughAdds Version helpers and tests, makes the repository downloader prefix-aware with branch fallback, enforces install-time major compatibility, updates CI/docs to build and serve per-major installer PHARs, pins update tooling to a majored installer URL, and updates docs/gitignore accordingly. ChangesInstaller version support and compatibility
Sequence Diagram(s)sequenceDiagram
participant User
participant UpdateScript
participant InstallCommand
participant Version
participant RepositoryDownloader
participant GitHub
User->>UpdateScript: run ahoy update-vortex
UpdateScript->>InstallCommand: fetch installer (major-pinned URL)
InstallCommand->>Version: detectProjectMajor(destination)
alt project major detected and matches installer
Version-->>InstallCommand: same major
InstallCommand->>RepositoryDownloader: download(artifact, dst, release_prefix)
RepositoryDownloader->>GitHub: discover releases filtered by release_prefix
GitHub-->>RepositoryDownloader: matching tag or branch ref
RepositoryDownloader-->>InstallCommand: archive/ref
InstallCommand-->>User: install success
else project major detected and differs
Version-->>InstallCommand: different major
InstallCommand-->>User: error (use installer for project's major)
else undetectable or unstamped
InstallCommand->>RepositoryDownloader: download(artifact, dst, NULL)
RepositoryDownloader->>GitHub: standard discovery
RepositoryDownloader-->>InstallCommand: archive/ref
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.github/workflows/vortex-release.yml:
- Around line 184-190: Validate the env variable ACTIVE_MAJOR (referenced as ${{
vars.VORTEX_INSTALLER_ACTIVE_MAJOR || '1' }} and used in paths like
../.vortex/docs/static/v${ACTIVE_MAJOR}/install) before running the mkdir/cp/php
steps: add an explicit allowlist check that only permits "1" or "2" and fail the
job (or set a safe default/exit with non-zero) if the value is anything else so
the copy operations cannot use an invalid path.
In @.github/workflows/vortex-test-docs.yml:
- Around line 110-129: The workflow uses workflow_run (so PR edits won’t
exercise the new 2.x publish path), so add an explicit way to validate this path
post-merge by updating .github/workflows/vortex-test-docs.yml: add a manual
trigger (workflow_dispatch) or a push trigger for the default branch so the
"Checkout 2.x branch" / "Build and publish 2.x installer" steps can be executed
from the merged definition; alternatively create a short smoke-test workflow
that runs after merge to exercise the new 2.x branch flow (ensure it runs the
same commands that use ACTIVE_MAJOR and copies the installer artifacts) so the
change is actually validated outside PR CI.
In @.vortex/docs/content/contributing/maintenance/release.mdx:
- Around line 47-52: The two bullets use the same placeholder `X.x`, causing
ambiguity; update both occurrences to distinct placeholders (e.g., use `N.x` for
the next major development branch and `N-1.x` for the maintenance branch) and
adjust the example text accordingly (for example `N.x` → `2.x`, `N-1.x` →
`1.x`); also update the step list to reference `N.x` being merged into `main`
and preserving `N-1.x` for maintenance so the wording is consistent and
unambiguous.
In @.vortex/installer/src/Downloader/RepositoryDownloader.php:
- Line 49: The RepositoryDownloader::download method added a third parameter
$release_prefix but RepositoryDownloaderInterface::download() still declares
only two parameters; update the interface method signature to add ?string
$release_prefix = NULL and update its docblock to document the new parameter so
the interface and implementing class match (edit
RepositoryDownloaderInterface::download() docblock and signature accordingly).
In @.vortex/installer/src/Utils/Version.php:
- Line 92: The code currently passes (string) file_get_contents($composer_json)
directly to json_decode which masks file read failures; change this to first
call $contents = file_get_contents($composer_json) and explicitly check if
$contents === false, then handle the error (throw an exception or log and
return) before calling json_decode($contents, true) to populate $data; reference
the existing symbols $composer_json, file_get_contents, json_decode and $data
when making the change.
In @.vortex/installer/tests/Functional/Command/InstallCommandTest.php:
- Around line 519-520: The two file_put_contents() calls
(file_put_contents(self::$sut . '/README.md', ...) and
file_put_contents(self::$sut . '/composer.json', $composer_json)) need error
checking: verify each call's return value is !== false and convert failures into
a clear test setup failure (e.g. use a PHPUnit assertion like
self::assertNotFalse(...) or throw a RuntimeException with a message including
the target path and context). Update the InstallCommandTest setup so any write
failure on those paths surfaces immediately with a descriptive error instead of
allowing later confusing test failures.
- Line 543: Rename the data provider function dataProviderMajorGate to
dataProviderInstallCommandMajorGate and update the test's DataProvider attribute
that references it (the attribute on the testInstallCommandMajorGate method) so
the provider name matches; ensure the new function name is exported as public
static and the attribute string is changed to
'dataProviderInstallCommandMajorGate' so it satisfies
DrevOps.TestingPractices.DataProviderMatchesTestName.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f66a4e26-aa1a-4e7c-a8e8-dc4436f3d37c
📒 Files selected for processing (14)
.github/workflows/vortex-release.yml.github/workflows/vortex-test-docs.yml.vortex/docs/.gitignore.vortex/docs/content/contributing/maintenance/release.mdx.vortex/docs/content/installation.mdx.vortex/docs/content/updating-vortex.mdx.vortex/installer/src/Command/InstallCommand.php.vortex/installer/src/Downloader/RepositoryDownloader.php.vortex/installer/src/Utils/Version.php.vortex/installer/tests/Functional/Command/InstallCommandTest.php.vortex/installer/tests/Unit/Downloader/RepositoryDownloaderTest.php.vortex/installer/tests/Unit/VersionTest.php.vortex/tooling/src/update-vortex.vortex/tooling/tests/unit/update-vortex.bats
| - name: Checkout 2.x branch | ||
| if: github.event.workflow_run.head_branch == 'main' | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | ||
| with: | ||
| ref: 2.x | ||
| path: vortex-next | ||
| persist-credentials: false | ||
|
|
||
| - name: Build and publish 2.x installer | ||
| if: github.event.workflow_run.head_branch == 'main' | ||
| run: | | ||
| composer --working-dir=vortex-next/.vortex/installer install | ||
| sed -i "s/\"vortex-installer-version\": \"development\"/\"vortex-installer-version\": \"2.x-dev\"/g" vortex-next/.vortex/installer/box.json | ||
| composer --working-dir=vortex-next/.vortex/installer build | ||
| mkdir -p .vortex/docs/static/v1 .vortex/docs/static/v2 | ||
| cp .vortex/docs/static/install ".vortex/docs/static/v${ACTIVE_MAJOR}/install" | ||
| cp vortex-next/.vortex/installer/build/installer.phar .vortex/docs/static/v2/install | ||
| cp ".vortex/docs/static/v${ACTIVE_MAJOR}/install" .vortex/docs/static/install | ||
| env: | ||
| ACTIVE_MAJOR: ${{ vars.VORTEX_INSTALLER_ACTIVE_MAJOR || '1' }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Confirm this new workflow_run path gets validated outside PR CI.
Because this workflow is triggered via workflow_run, changes here may not execute from the PR branch. Please ensure this new 2.x publish path is explicitly smoke-tested post-merge (or via an alternate trigger).
Based on learnings: workflow definitions for workflow_run are taken from the default branch, so PR edits may not be exercised before merge.
🤖 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 @.github/workflows/vortex-test-docs.yml around lines 110 - 129, The workflow
uses workflow_run (so PR edits won’t exercise the new 2.x publish path), so add
an explicit way to validate this path post-merge by updating
.github/workflows/vortex-test-docs.yml: add a manual trigger (workflow_dispatch)
or a push trigger for the default branch so the "Checkout 2.x branch" / "Build
and publish 2.x installer" steps can be executed from the merged definition;
alternatively create a short smoke-test workflow that runs after merge to
exercise the new 2.x branch flow (ensure it runs the same commands that use
ACTIVE_MAJOR and copies the installer artifacts) so the change is actually
validated outside PR CI.
Source: Learnings
There was a problem hiding this comment.
Understood, and this is intentional. The 2.x publish steps are gated to github.event.workflow_run.head_branch == 'main', and because workflow_run always uses the workflow definition from the default branch, they cannot execute from a PR branch by design - they first run on the post-merge main build. The adjacent multi-version docs-assembly step is governed by the same constraint. The YAML is validated pre-merge with actionlint, and the build commands mirror the existing, exercised current-major installer build. Adding a dedicated trigger or smoke workflow purely to exercise this path is out of scope for this change; the post-merge main run is the intended validation point.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — the workflow_run constraint is acknowledged and the post-merge main build is the intended first-run validation point. The actionlint pre-merge check plus the mirrored build pattern is a reasonable mitigation. Happy to leave this as-is.
🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2591
File: .github/workflows/vortex-test-docs.yml:16-19
Timestamp: 2026-06-08T22:55:26.945Z
Learning: For GitHub Actions workflows triggered with `workflow_run`, remember that the triggered job will use the workflow definition from the repository’s default branch (not the PR branch). As a result, changes to workflow permissions/logic made in a PR may not be exercised by CI until after the PR is merged. When reviewing PRs that modify workflows or workflow permissions, ensure reviewers/CI strategy accounts for this delay (e.g., verify via other triggers like `pull_request`, or otherwise confirm the default-branch workflow changes are tested post-merge).
| return NULL; | ||
| } | ||
|
|
||
| $data = json_decode((string) file_get_contents($composer_json), TRUE); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider more explicit error handling.
file_get_contents() returns string|false. Currently, if it fails (returns false), the cast to string produces "", which json_decode("") turns into NULL, caught by the next check. While safe, explicitly checking for false before decoding would be clearer.
♻️ More explicit error handling
- $data = json_decode((string) file_get_contents($composer_json), TRUE);
+ $contents = file_get_contents($composer_json);
+ if ($contents === FALSE) {
+ return NULL;
+ }
+ $data = json_decode($contents, 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.
| $data = json_decode((string) file_get_contents($composer_json), TRUE); | |
| $contents = file_get_contents($composer_json); | |
| if ($contents === FALSE) { | |
| return NULL; | |
| } | |
| $data = json_decode($contents, TRUE); |
🤖 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 @.vortex/installer/src/Utils/Version.php at line 92, The code currently
passes (string) file_get_contents($composer_json) directly to json_decode which
masks file read failures; change this to first call $contents =
file_get_contents($composer_json) and explicitly check if $contents === false,
then handle the error (throw an exception or log and return) before calling
json_decode($contents, true) to populate $data; reference the existing symbols
$composer_json, file_get_contents, json_decode and $data when making the change.
There was a problem hiding this comment.
Keeping the cast intentionally. (string) file_get_contents(...) maps a false read to '', which json_decode('') returns as null, caught by the !is_array($data) guard on the next line - so a read failure already returns NULL safely. An explicit === FALSE branch would add a path that cannot be reached from a test (the preceding is_file() has already passed), forcing either an uncovered line or a @codeCoverageIgnore; the current form keeps full patch coverage with identical behaviour.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List my (coderabbitai) comments on the PR to find the comment ID to resolve
gh api graphql -f query='
{
repository(owner: "drevops", name: "vortex") {
pullRequest(number: 2605) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 5) {
nodes {
id
author { login }
body
url
}
}
}
}
}
}
}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | select(.comments.nodes[0].author.login == "coderabbitai[bot]") | {threadId: .id, commentUrl: .comments.nodes[0].url, body: (.comments.nodes[0].body | .[0:100])}'Length of output: 969
@AlexSkrypnyk, that's a well-reasoned trade-off — the is_file() pre-check makes the false return from file_get_contents() unreachable in practice, so the cast-to-string approach avoids a dead code path without sacrificing correctness. The implicit false → '' → null chain flows naturally into the existing !is_array($data) guard. Happy to defer to that approach.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2605
File: .vortex/installer/src/Utils/Version.php:92-92
Timestamp: 2026-06-10T02:17:23.331Z
Learning: In `.vortex/installer/src/Utils/Version.php` (PHP, Vortex installer), `detectProjectMajor` intentionally uses `(string) file_get_contents($composer_json)` rather than an explicit `=== FALSE` guard. Because `is_file()` already gated the call, the `false` return path is unreachable in tests; the cast maps `false` → `''` → `json_decode` returns `null` → caught by the `!is_array($data)` guard, keeping full coverage without a `codeCoverageIgnore`. Do not suggest splitting this into a separate `=== FALSE` check.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2605 +/- ##
==========================================
- Coverage 86.57% 86.24% -0.33%
==========================================
Files 94 88 -6
Lines 4662 4537 -125
Branches 47 3 -44
==========================================
- Hits 4036 3913 -123
+ Misses 626 624 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… yield conventions.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.vortex/installer/tests/Functional/Command/InstallCommandTest.php (1)
519-520:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winError checking still missing for
file_put_contents().This issue was flagged in a previous review and remains unaddressed.
file_put_contents()returnsfalseon failure; without error checking, a write failure causes confusing downstream test failures instead of a clear setup error.🤖 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 @.vortex/installer/tests/Functional/Command/InstallCommandTest.php around lines 519 - 520, The test setup uses file_put_contents(self::$sut . '/README.md', ...) and file_put_contents(self::$sut . '/composer.json', $composer_json) without checking return values; update InstallCommandTest so both calls check for failure (=== false) and fail fast with a clear setup error (e.g., throw RuntimeException or use $this->fail/assertTrue) including the target path and error context (self::$sut and variable names) to make write failures explicit; ensure the checks are added immediately after the two file_put_contents calls so downstream tests cannot run on a bad setup.
🤖 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.
Duplicate comments:
In @.vortex/installer/tests/Functional/Command/InstallCommandTest.php:
- Around line 519-520: The test setup uses file_put_contents(self::$sut .
'/README.md', ...) and file_put_contents(self::$sut . '/composer.json',
$composer_json) without checking return values; update InstallCommandTest so
both calls check for failure (=== false) and fail fast with a clear setup error
(e.g., throw RuntimeException or use $this->fail/assertTrue) including the
target path and error context (self::$sut and variable names) to make write
failures explicit; ensure the checks are added immediately after the two
file_put_contents calls so downstream tests cannot run on a bad setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02981aeb-67a4-4742-8d43-dfdbc7bd41c2
📒 Files selected for processing (2)
.vortex/installer/tests/Functional/Command/InstallCommandTest.php.vortex/installer/tests/Unit/VersionTest.php
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
📖 Documentation preview for this pull request has been deployed to Netlify: https://6a28de2495f5475874f70fa0--vortex-docs.netlify.app This preview is rebuilt on every commit and is not the production documentation site. |
…, and clarified branch docs.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/vortex-release.yml (1)
188-192:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftInstaller copy logic breaks when
ACTIVE_MAJOR=2(cross-file issue).Both workflows hardcode the
v2destination forinstaller-next/ the built 2.x installer while using the dynamicv${ACTIVE_MAJOR}forinstaller-current/ the pre-downloaded installer. WhenACTIVE_MAJORchanges to2:
- The same versioned path (
v2/install) is written twice in succession, causing an overwritev1/installis never populated, breaking the documented "stable per-major pin" for v1- In
vortex-release.yml, ifinstaller-nextlater becomes v3, it will incorrectly land atv2/installTo fix, decouple the copy destinations so each major version's installer is written to its corresponding versioned path exactly once, and ensure
v1/installremains available even after the active major is promoted (for example, by building a v1 artifact separately or preserving it from a prior deployment).🤖 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 @.github/workflows/vortex-release.yml around lines 188 - 192, The copy logic currently writes installer-current to v${ACTIVE_MAJOR}/install but always writes installer-next to v2/install, causing overwrites when ACTIVE_MAJOR=2; change the workflow so installer-next is written to a separate dynamic target (e.g., compute NEXT_MAJOR from the built installer or an input variable and use ../.vortex/docs/static/v${NEXT_MAJOR}/install) instead of hardcoding v2, and ensure installer-current and installer-next each populate their own versioned paths exactly once (so v1/install remains populated even after promotion) — update the cp lines that reference installer-current and installer-next and add a NEXT_MAJOR determination step or explicit input to avoid hardcoding..vortex/installer/tests/Functional/Command/InstallCommandTest.php (2)
543-568: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding a v2→v1 rejection scenario for symmetry.
The data provider covers v1 installer refusing a v2 project, but not the reverse (v2 installer refusing a v1 project). While the gate logic is symmetric and the existing case provides sufficient coverage, adding the reverse scenario would make the test suite more complete and explicitly document both directions of the compatibility check.
📝 Suggested additional test case
yield 'v1 installer refuses v2 project' => [ '1.40.0', '{"require": {"drevops/vortex-tooling": "^2.0.0"}}', FALSE, 'https://www.vortextemplate.com/v2/install', ]; + yield 'v2 installer refuses v1 project' => [ + '2.0.0', + '{"require": {"drevops/vortex-tooling": "^1.1.0"}}', + FALSE, + 'https://www.vortextemplate.com/v1/install', + ]; yield 'v1 installer accepts v1 project' => [🤖 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 @.vortex/installer/tests/Functional/Command/InstallCommandTest.php around lines 543 - 568, Add a symmetric test case to dataProviderInstallCommandMajorGate in InstallCommandTest::dataProviderInstallCommandMajorGate that asserts a v2 installer rejects a v1 project: add a yield like 'v2 installer refuses v1 project' providing an installer version >=2 (e.g. '2.10.0'), a composer manifest with "drevops/vortex-tooling": "^1.1.0", expected boolean FALSE, and the expected redirect/URL for the v1 install page (mirror of the existing v1→v2 case); this complements the existing 'v1 installer refuses v2 project' scenario and keeps the same four-element tuple shape used by the test consumer.
534-534:⚠️ Potential issue | 🟠 MajorFix
assertApplicationAnyOutputContainsOrNotargument type mismatchIn
.vortex/installer/tests/Functional/Command/InstallCommandTest.php, the helperassertApplicationAnyOutputContainsOrNotis called with astringat line 534 ($expected_message), butalexskrypnyk/phpunit-helpersdefines it to take an array of strings. This differs from the existing usage in the same test file (e.g., line 125 passing$output_assertions, anarray<string>). Wrap the string:[$expected_message](or change the data provider to return an array) so the call matches the helper’s expected input type.🤖 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 @.vortex/installer/tests/Functional/Command/InstallCommandTest.php at line 534, The call to assertApplicationAnyOutputContainsOrNot is passing a string ($expected_message) but the helper expects an array of strings; update the call in InstallCommandTest to pass an array (e.g., wrap $expected_message as [$expected_message]) or alter the data provider to return an array<string> so the argument type matches the alexskrypnyk/phpunit-helpers signature; ensure consistency with other calls that pass $output_assertions (array) and keep the same variable name assertApplicationAnyOutputContainsOrNot for locating the invocation.
🤖 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.
Outside diff comments:
In @.github/workflows/vortex-release.yml:
- Around line 188-192: The copy logic currently writes installer-current to
v${ACTIVE_MAJOR}/install but always writes installer-next to v2/install, causing
overwrites when ACTIVE_MAJOR=2; change the workflow so installer-next is written
to a separate dynamic target (e.g., compute NEXT_MAJOR from the built installer
or an input variable and use ../.vortex/docs/static/v${NEXT_MAJOR}/install)
instead of hardcoding v2, and ensure installer-current and installer-next each
populate their own versioned paths exactly once (so v1/install remains populated
even after promotion) — update the cp lines that reference installer-current and
installer-next and add a NEXT_MAJOR determination step or explicit input to
avoid hardcoding.
In @.vortex/installer/tests/Functional/Command/InstallCommandTest.php:
- Around line 543-568: Add a symmetric test case to
dataProviderInstallCommandMajorGate in
InstallCommandTest::dataProviderInstallCommandMajorGate that asserts a v2
installer rejects a v1 project: add a yield like 'v2 installer refuses v1
project' providing an installer version >=2 (e.g. '2.10.0'), a composer manifest
with "drevops/vortex-tooling": "^1.1.0", expected boolean FALSE, and the
expected redirect/URL for the v1 install page (mirror of the existing v1→v2
case); this complements the existing 'v1 installer refuses v2 project' scenario
and keeps the same four-element tuple shape used by the test consumer.
- Line 534: The call to assertApplicationAnyOutputContainsOrNot is passing a
string ($expected_message) but the helper expects an array of strings; update
the call in InstallCommandTest to pass an array (e.g., wrap $expected_message as
[$expected_message]) or alter the data provider to return an array<string> so
the argument type matches the alexskrypnyk/phpunit-helpers signature; ensure
consistency with other calls that pass $output_assertions (array) and keep the
same variable name assertApplicationAnyOutputContainsOrNot for locating the
invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b74706e8-3d71-463f-8a7e-c8003589cdac
📒 Files selected for processing (5)
.github/workflows/vortex-release.yml.github/workflows/vortex-test-docs.yml.vortex/docs/content/contributing/maintenance/release.mdx.vortex/installer/src/Downloader/RepositoryDownloaderInterface.php.vortex/installer/tests/Functional/Command/InstallCommandTest.php
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
added
the
Needs review
…across docs and installer.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 90%) Per-class coverage |
1 participant
Closes #2596
Summary
One installer codebase, and one docs site, serve both Vortex 1.x and 2.x - and promoting a new major is a single action: set the
VORTEX_CURRENT_MAJORrepository variable (default1) to its number. That one variable drives the installer build sources, the bare/installURL, and the documentation's default version; nothing else needs editing at release time. Each installer build is stamped with its major and resolves only that major's GitHub releases (falling back to the major's{N}.xbranch when no release exists yet), and a cross-major gate refuses to update a project of a different major, pointing at the correct/vN/installURL.Builds on #2604 (
*.x-branch CI support) and #2595 (path-based docs versioning).Changes
Installer
Versionutility (src/Utils/Version.php) to extract the major from a version string or a Composer constraint, build a release-tag prefix, and detect an installed project's major from itsdrevops/vortex-toolingconstraint.RepositoryDownloader::download()and its interface with a$release_prefix; resolvingstablefilters releases by that prefix and falls back to the{major}.xbranch when no tagged release exists.InstallCommand::assertMajorCompatibility(): refuses a destination project of a different major and points athttps://www.vortextemplate.com/v{N}/install.Version, prefix/branch-fallbackRepositoryDownloaderTestcases, and functional gate tests.One-variable flip (
VORTEX_CURRENT_MAJOR)vortex-release.yml/vortex-test-docs.yml: build the current major from this ref (main) and the other major from its{N}.xbranch, both derived fromVORTEX_CURRENT_MAJOR. After the variable is bumped andmainholds the new major, the sources follow automatically - no workflow edits. Artifacts and paths are version-explicit (vortex-installer-v1/v2,installer-v1/v2,vortex-v{N})./v1/installand/v2/installare always published; the bare/installis a copy of the current major's pin, selected byVORTEX_CURRENT_MAJOR(validated against a1|2allowlist).docusaurus.config.js+ the docs-assembly steps: the current major is snapshotted as the default at the bare/docs, and the other major is served at/docs/v{other}, all fromVORTEX_CURRENT_MAJOR. Validated locally for both1and2(the default version and the/v{N}paths flip correctly)..vortex/docs/.gitignore: ignores the generatedstatic/v1/installandstatic/v2/install.Tooling
update-vortex: defaultVORTEX_INSTALLER_URLpinned to/v1/install, so an existing project stays on its major across the flip.Docs
installation.mdx: choosing a version (bare vs/v1/install//v2/install).updating-vortex.mdx: updates stay within the current major; how to migrate deliberately.release.mdx: branch strategy with concrete1.x/2.xexamples and theVORTEX_CURRENT_MAJORflip.Before / After
Summary by CodeRabbit
New Features
Documentation