[#2044] Allow to select versioning schema in installer. #2113
Conversation
WalkthroughAdds a selectable release versioning scheme (CalVer, SemVer, Other) to the installer, persists the choice to Changes
Sequence Diagram(s)sequenceDiagram
participant User as Installer User
participant PM as PromptManager
participant VS as VersionScheme Handler
participant ENV as .env
participant FS as Filesystem
User->>PM: runPrompts()
PM->>VS: discover()
VS->>ENV: read VORTEX_RELEASE_VERSION_SCHEME
ENV-->>VS: current value / null
VS-->>PM: discovered default
PM->>User: present options (CalVer / SemVer / Other)
User->>PM: selects scheme
PM->>VS: process(selection)
VS->>VS: determine token removals
alt selection == SEMVER
VS->>FS: remove CALVER fenced content
else selection == CALVER
VS->>FS: remove SEMVER fenced content
else selection == OTHER
VS->>FS: remove both CALVER & SEMVER fenced content
end
VS->>ENV: write VORTEX_RELEASE_VERSION_SCHEME
ENV-->>VS: persisted
VS-->>PM: complete
PM-->>User: continue install flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
AlexSkrypnyk
force-pushed
the
feature/2044-allow-versioning-schema
branch
from
680c034 to
da77beb
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2113 +/- ##
===========================================
+ Coverage 70.38% 70.59% +0.21%
===========================================
Files 98 99 +1
Lines 4980 5023 +43
Branches 44 44
===========================================
+ Hits 3505 3546 +41
- Misses 1475 1477 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
193-223: Add VersionScheme todefaultTuiAnswers()to keep TUI tests in syncThe new VersionScheme prompt is now part of
runPrompts(), butdefaultTuiAnswers()doesn’t provide a default answer for it. Any tests that rely solely ondefaultTuiAnswers()+tuiKeystrokes()are likely to run out of keystrokes or behave unpredictably when the VersionScheme select is shown.Recommend adding a default entry, positioned alongside the other Code repository prompts:
public static function defaultTuiAnswers(): array { return [ @@ Theme::id() => static::TUI_DEFAULT, ThemeCustom::id() => static::TUI_DEFAULT, CodeProvider::id() => static::TUI_DEFAULT, + VersionScheme::id() => static::TUI_DEFAULT, Timezone::id() => static::TUI_DEFAULT, Services::id() => static::TUI_DEFAULT, Tools::id() => static::TUI_DEFAULT,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (36)
.vortex/docs/static/img/installer.svgis excluded by!**/*.svg.vortex/installer/tests/Fixtures/install/_baseline/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.github/workflows/draft-release-notes.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_ftp/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_all_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_all_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_artifact/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_container_image/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_none_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_none_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_project_name___acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_project_name___lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_all/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_github_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_jira_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_newrelic_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_slack_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_webhook_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/starter_drupal_cms_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/starter_drupal_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_calver/.ignorecontentis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_other/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_other/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_semver/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_semver/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (11)
.env(1 hunks).github/workflows/draft-release-notes.yml(1 hunks).vortex/docs/content/workflows/releasing.mdx(1 hunks).vortex/docs/content/workflows/variables.mdx(1 hunks).vortex/docs/cspell.json(3 hunks).vortex/installer/src/Prompts/Handlers/VersionScheme.php(1 hunks).vortex/installer/src/Prompts/PromptManager.php(5 hunks).vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(2 hunks).vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php(1 hunks)docs/releasing.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php
🧬 Code graph analysis (5)
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(51-279)dataProviderRunPrompts(116-116)getExpectedDefaults(121-153)getExpectedInstalled(158-169)stubVortexProject(265-271)stubDotenvValue(257-263)
.vortex/installer/src/Prompts/PromptManager.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62)
.vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(62-638)makeEnvName(456-458).vortex/installer/src/Utils/Env.php (2)
Env(7-247)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (2)
assertSutContains(113-128)assertSutNotContains(130-145)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (4)
.vortex/installer/src/Utils/Env.php (3)
Env(7-247)getFromDotenv(28-43)writeValueDotenv(142-202).vortex/installer/src/Utils/File.php (2)
File(10-121)removeTokenAsync(93-100).vortex/installer/src/Utils/Tui.php (3)
Tui(15-259)bold(81-83)underscore(85-87).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (2)
AbstractHandler(10-233)getResponseAsString(191-197)
🪛 LanguageTool
.vortex/docs/content/workflows/releasing.mdx
[style] ~37-~37: To form a complete sentence, be sure to include a subject.
Context: ...est development code, not yet released. May be unstable, but CI tests should pass. ...
(MISSING_IT_THERE)
docs/releasing.md
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...release/X.Y.Zin GitHub repository. 2. Release tag exists asX.Y.Z` in GitHub reposit...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
.vortex/docs/content/workflows/variables.mdx
[style] ~1987-~1987: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... Versioning scheme used for releases. Can be one of: calver, semver, other
@s...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.18.1)
docs/releasing.md
9-9: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (9)
- GitHub Check: deploy
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
🔇 Additional comments (5)
.env (1)
219-227: New RELEASE VERSIONING section is consistent and well-documentedThe
VORTEX_RELEASE_VERSION_SCHEMEvariable, its allowed values, and defaultcalveralign with the installer handler and releasing docs. The@seelink ties configuration and documentation together cleanly..vortex/docs/content/workflows/releasing.mdx (1)
178-244: Version Scheme and configuration docs align with installer and env changesThe “Version Scheme”, CalVer/SemVer/Other descriptions, and the
.envexample forVORTEX_RELEASE_VERSION_SCHEMEare clear and consistent with the installer prompt and workflow behaviour. This gives users a good mental model of how their choice propagates through releases and CI..vortex/docs/cspell.json (1)
14-16: Dictionary updates correctly cover new release/versioning terminologyAdding “Runsheet(s)”, “calver”, “hotfixes”, and their lowercase variants matches the new releasing documentation and avoids spurious spell-check noise.
Also applies to: 22-22, 40-40, 66-67
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
36-37: Default expectation for VersionScheme matches installer behaviourImporting
VersionSchemeand expectingVersionScheme::CALVERingetExpectedDefaults()keeps unit tests aligned with the installer’s default and the.envdefault.Also applies to: 121-152
.vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php (1)
16-51: Functional coverage cleanly verifies tokenized content for each schemeThe three datasets nicely exercise CALVER, SEMVER, and OTHER selections via the prompt env var and assert the expected presence/absence of versioning text and links in the generated SUT. This should catch regressions in token handling or prompt wiring.
| - name: Load environment variables from .env | ||
| run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" | ||
|
|
||
| - name: Generate CalVer version | ||
| if: vars.VORTEX_RELEASE_VERSION_SCHEME == 'calver' | ||
| if: ${{ contains(env.VORTEX_RELEASE_VERSION_SCHEME, 'calver') }} | ||
| id: calver |
There was a problem hiding this comment.
🧩 Analysis chain
Sourcing .env without checkout and exporting full env to GITHUB_ENV is risky
This step will likely fail and is broader than needed:
- On GitHub-hosted runners the repo is not checked out by default. Without a preceding
actions/checkout,. ./.envwill fail because.envisn’t present. env >> "$GITHUB_ENV"re-exports the entire process environment, which is unnecessary for this workflow and could have subtle side-effects (especially if any variable contains unexpected characters or newlines).
I’d recommend:
- Ensure the repository is checked out before sourcing
.env. - Limit what you write to
$GITHUB_ENVto the specific variables you actually need (here,VORTEX_RELEASE_VERSION_SCHEME).
Example adjustment:
steps:
+ - name: Checkout repository
+ uses: actions/checkout@v4
+
- name: Load environment variables from .env
- run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV"
+ run: |
+ set -a
+ . ./.env
+ set +a
+ # Only export the variables this workflow cares about.
+ env | grep '^VORTEX_RELEASE_VERSION_SCHEME=' >> "$GITHUB_ENV"
@@
- - name: Generate CalVer version
- if: ${{ contains(env.VORTEX_RELEASE_VERSION_SCHEME, 'calver') }}
+ - name: Generate CalVer version
+ if: ${{ contains(env.VORTEX_RELEASE_VERSION_SCHEME, 'calver') }}🏁 Script executed:
#!/bin/bash
# Examine the draft-release-notes.yml workflow file to verify step ordering
cat .github/workflows/draft-release-notes.ymlLength of output: 1077
🏁 Script executed:
#!/bin/bash
# Check if .env file exists in repo root
ls -la .env 2>&1 || echo "File not found at repo root"Length of output: 54
🏁 Script executed:
#!/bin/bash
# Search for any checkout steps in the workflow
rg -n "checkout" .github/workflows/draft-release-notes.ymlLength of output: 0
Add actions/checkout before sourcing .env and export only needed variables
The step will fail because GitHub-hosted runners do not auto-checkout the repository. Without a preceding actions/checkout step, the working directory won't contain .env, and . ./.env will fail with "No such file or directory."
Additionally, env >> "$GITHUB_ENV" exports the entire process environment to the workflow, which is unnecessarily broad and could introduce unexpected side effects.
Recommended fix:
steps:
+ - name: Checkout repository
+ uses: actions/checkout@v4
+
- name: Load environment variables from .env
- run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV"
+ run: |
+ set -a
+ . ./.env
+ set +a
+ env | grep '^VORTEX_RELEASE_VERSION_SCHEME=' >> "$GITHUB_ENV"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/draft-release-notes.yml around lines 22 to 27: the job
attempts to source ./.env before the repository is checked out and then dumps
the entire process environment into $GITHUB_ENV, which will fail and over-export
variables; add an actions/checkout step immediately before the "Load environment
variables from .env" step so .env exists, and change the export logic to only
append the specific variables you need from the .env file to $GITHUB_ENV (e.g.,
read or filter the .env entries and echo only the required keys) instead of
using env >> "$GITHUB_ENV".
| - **Development** - Latest development code, not yet released. May be unstable, but CI tests should pass. | ||
| - **Stage** - Pre-production environment. Mirrors production as closely as possible. Used for final release testing. | ||
| - **Production** - Live customer-facing application. Stable and reliable. Source of truth for data. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten environment bullet phrasing to avoid fragments
The “Development / Stage / Production” bullets read slightly fragmented (“May be unstable, but CI tests should pass.”, “Used for final release testing.”). Consider making them full sentences for readability and to satisfy style linters, e.g.:
- **Development** – Latest development code, not yet released. It may be unstable, but CI tests should still pass.
- **Stage** – Pre-production environment that mirrors production as closely as possible. It is used for final release testing.
- **Production** – Live customer-facing application that must remain stable and reliable. It is the primary source of truth for data.🧰 Tools
🪛 LanguageTool
[style] ~37-~37: To form a complete sentence, be sure to include a subject.
Context: ...est development code, not yet released. May be unstable, but CI tests should pass. ...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
.vortex/docs/content/workflows/releasing.mdx around lines 37 to 39: the three
environment bullets are written as fragments; rewrite each bullet as a complete
sentence to improve readability and satisfy style linters by turning fragments
into full sentences (e.g., prepend subjects like "It" or "This environment" and
make explicit predicates so each line reads as a standalone sentence while
preserving meaning about Development, Stage, and Production).
| ### `VORTEX_RELEASE_VERSION_SCHEME` | ||
|
|
||
| Versioning scheme used for releases. | ||
|
|
||
| Can be one of: calver, semver, other<br/>@see https://www.vortextemplate.com/docs/workflows/releasing | ||
|
|
||
| Default value: `calver` | ||
|
|
||
| Defined or used in: `.env` |
There was a problem hiding this comment.
Clarify description sentence and include all usage locations for the variable
The description reads a bit fragmentary and the “Defined or used in” list appears incomplete given this variable also drives CI and installer behavior. Consider:
- Rewording the description to a full sentence, e.g. “The versioning scheme used for releases.”
- Updating the usage line to reflect all relevant locations, e.g. “
.env, CI config, installer” (or whatever files actually read it) to stay consistent with how other variables are documented.
🧰 Tools
🪛 LanguageTool
[style] ~1987-~1987: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... Versioning scheme used for releases. Can be one of: calver, semver, other
@s...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
.vortex/docs/content/workflows/variables.mdx around lines 1984 to 1992: the
description for VORTEX_RELEASE_VERSION_SCHEME is fragmentary and the "Defined or
used in" list is incomplete; change the short fragment into a full sentence like
"The versioning scheme used for releases." and update the usage line to list all
places this variable is read (e.g., ".env, CI configuration, installer" —
replace with the exact file names/locations if different) so it matches the
style of other variables.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function label(): string { | ||
| return 'Release versioning schema'; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align terminology (“scheme” vs “schema”) and optionally sync CalVer example with docs
Implementation and wiring (constants, default CALVER, discovery from .env, token handling) look correct and consistent with the docs and tests.
Only minor UX/text nits:
- The label/description use “versioning schema” while the hint and env var name use “version scheme”. It’d be cleaner to pick one term (probably “scheme”, to match
VORTEX_RELEASE_VERSION_SCHEME) and use it consistently across label/description/hint. - The CalVer example in the prompt (
24.1.0) differs from the 25.x examples in the updated releasing docs; consider updating to a 25.x value for consistency, unless 24.x is intentional.
Also applies to: 29-53, 59-61
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/VersionScheme.php lines 19-24 (and also
check lines 29-53, 59-61): the prompt text mixes “schema” and “scheme” and uses
a CalVer example out of sync with docs; update all label/description/hint
strings and any inline references to consistently use “scheme” (to match
VORTEX_RELEASE_VERSION_SCHEME and related env var names), and change the CalVer
example from 24.x to a 25.x value (e.g. 25.1.0) everywhere in this file so
wording and examples are uniform.
| public static function dataProviderRunPrompts(): array { | ||
| $expected_defaults = static::getExpectedDefaults(); | ||
| $expected_installed = static::getExpectedInstalled(); | ||
|
|
||
| return [ | ||
| 'version scheme - prompt' => [ | ||
| [VersionScheme::id() => Key::ENTER], | ||
| $expected_defaults, | ||
| ], | ||
|
|
||
| 'version scheme - discovery - calver' => [ | ||
| [], | ||
| [VersionScheme::id() => VersionScheme::CALVER] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| $test->stubDotenvValue('VORTEX_RELEASE_VERSION_SCHEME', VersionScheme::CALVER); | ||
| }, | ||
| ], | ||
|
|
||
| 'version scheme - discovery - semver' => [ | ||
| [], | ||
| [VersionScheme::id() => VersionScheme::SEMVER] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| $test->stubDotenvValue('VORTEX_RELEASE_VERSION_SCHEME', VersionScheme::SEMVER); | ||
| }, | ||
| ], | ||
|
|
||
| 'version scheme - discovery - other' => [ | ||
| [], | ||
| [VersionScheme::id() => VersionScheme::OTHER] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| $test->stubDotenvValue('VORTEX_RELEASE_VERSION_SCHEME', VersionScheme::OTHER); | ||
| }, | ||
| ], | ||
|
|
||
| 'version scheme - discovery - missing .env' => [ | ||
| [], | ||
| $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| }, | ||
| ], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unit coverage for VersionScheme prompt behavior looks solid; consider adding an invalid‑value scenario
The datasets cover the direct prompt path, discovery for each valid value, and the missing‑.env case, all wired through existing helpers, which is good.
As an extra safeguard, you might add a scenario where .env contains an invalid VORTEX_RELEASE_VERSION_SCHEME value and assert that discover() falls back to the default (CALVER). That would lock in the guard in VersionScheme::discover().
🤖 Prompt for AI Agents
In .vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php
around lines 15 to 60, add a new data set in dataProviderRunPrompts to cover an
invalid .env value: create a case (e.g. 'version scheme - discovery - invalid
value') that passes an empty input array, expects [VersionScheme::id() =>
VersionScheme::CALVER] + $expected_installed, and in the closure call
$test->stubVortexProject($config) then
$test->stubDotenvValue('VORTEX_RELEASE_VERSION_SCHEME', 'INVALID') (or similar
invalid string) so the test asserts discover() falls back to CALVER.
| 3. The `HEAD` of the `main` branch has `X.Y.Z` tag. | ||
| 4. The hash of the `HEAD` of the `main` branch exists in the `develop` branch. | ||
| This is to ensure that everything pushed to `main` exists in `develop` (in | ||
| case if `main` had any hot-fixes that not yet have been merged | ||
| to `develop`). | ||
| 5. There are no PRs in GitHub related to the release. | ||
| 6. The hash of the `HEAD` of the `production` branch matches the hash of | ||
| the `HEAD` of `master` branch. | ||
| the `HEAD` of `main` branch. | ||
|
|
There was a problem hiding this comment.
Tokenization matches installer logic; fix duplicate CalVer example and minor phrasing
The new VERSION_RELEASE_SCHEME_SEMVER / VERSION_RELEASE_SCHEME_CALVER markers line up with the handler logic in VersionScheme::process(), which is good.
Two small nits in the changed block:
- The “Incorrect” CalVer examples list contains
25.0.0twice; you can replace one with another invalid case (e.g.25.13or25.13.00) to avoid duplication. - The sentence about hot‑fixes reads a bit awkwardly:
“in case ifmainhad any hot-fixes that not yet have been merged todevelop”
Consider simplifying to something like “in casemainhad any hot‑fixes that have not yet been merged todevelop.”
Also applies to: 24-26, 41-45, 55-60
AlexSkrypnyk
force-pushed
the
feature/2044-allow-versioning-schema
branch
from
da77beb to
ce97f24
Compare
AlexSkrypnyk
force-pushed
the
feature/2044-allow-versioning-schema
branch
from
ce97f24 to
37be979
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/draft-release-notes.yml (1)
22-23: Addactions/checkoutbefore sourcing.env.Without a checkout step,
.envwon't exist in the runner's working directory, causing this step to fail..vortex/docs/content/workflows/variables.mdx (1)
1984-1992: Consider expanding "Defined or used in" to include CI configuration.The
.github/workflows/draft-release-notes.ymlworkflow also reads this variable. For completeness, consider:-Defined or used in: `.env` +Defined or used in: `.env`, `CI config`This matches the documentation pattern used for other variables that appear in both
.envand CI workflows.docs/releasing.md (1)
15-19: Clean up CalVer examples and release-outcome phrasing (tokens look correct)The
VERSION_RELEASE_SCHEME_*fences align withVersionScheme::process()token handling, so the conditional sections should behave as intended.Two small doc issues remain here, both previously flagged:
- The CalVer “Incorrect” list still contains
25.0.0twice; consider replacing one with another invalid pattern (e.g.25.13or25.13.00) to avoid duplication.- The explanation sentence under point 4 reads awkwardly:
“This is to ensure that everything pushed tomainexists indevelop(in case ifmainhad any hot-fixes that not yet have been merged todevelop).”
Something like “This ensures everything pushed tomainalso exists indevelop, in casemainhas hot-fixes that have not yet been merged back.” would read more naturally.Optionally, you could also wrap the bare URL on line 9 in Markdown link syntax to satisfy MD034 (e.g.
<https://www.vortextemplate.com/docs/workflows/releasing>).Also applies to: 55-59
.vortex/docs/content/workflows/releasing.mdx (1)
37-39: Consider rewriting environment bullets as complete sentences for clarityThe Development/Stage/Production bullets read clearly, but they still contain short fragments (“May be unstable...”, “Used for final release testing.”, “Stable and reliable. Source of truth for data.”). To improve readability and satisfy style linters, you could roll each bullet into a single full sentence, e.g.:
- “Latest development code, not yet released. It may be unstable, but CI tests should still pass.”
- “Pre-production environment that mirrors production as closely as possible. It is used for final release testing.”
- “Live customer-facing application that must remain stable and reliable. It is the primary source of truth for data.”
This is purely a style improvement; the technical content looks good.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (36)
.vortex/docs/static/img/installer.svgis excluded by!**/*.svg.vortex/installer/tests/Fixtures/install/_baseline/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.github/workflows/draft-release-notes.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_ftp/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_all_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_all_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_artifact/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_container_image/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_none_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_types_none_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_project_name___acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_project_name___lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_all/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_github_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_jira_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_newrelic_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_slack_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/notification_channels_webhook_only/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/starter_drupal_cms_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/starter_drupal_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_calver/.ignorecontentis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_other/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_other/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_semver/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/version_scheme_semver/docs/releasing.mdis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (11)
.env(1 hunks).github/workflows/draft-release-notes.yml(1 hunks).vortex/docs/content/workflows/releasing.mdx(1 hunks).vortex/docs/content/workflows/variables.mdx(1 hunks).vortex/docs/cspell.json(3 hunks).vortex/installer/src/Prompts/Handlers/VersionScheme.php(1 hunks).vortex/installer/src/Prompts/PromptManager.php(5 hunks).vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(3 hunks).vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php(1 hunks)docs/releasing.md(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php
🧬 Code graph analysis (5)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19)
.vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(62-638)makeEnvName(456-458).vortex/installer/src/Utils/Env.php (2)
Env(7-247)put(24-26)
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(51-280)dataProviderRunPrompts(116-116)getExpectedDefaults(121-153)getExpectedInstalled(158-169)stubVortexProject(266-272)stubDotenvValue(258-264)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (4)
.vortex/installer/src/Utils/Env.php (3)
Env(7-247)getFromDotenv(28-43)writeValueDotenv(142-202).vortex/installer/src/Utils/File.php (2)
File(10-121)removeTokenAsync(93-100).vortex/installer/src/Utils/Tui.php (3)
Tui(15-259)bold(81-83)underscore(85-87).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (2)
AbstractHandler(10-233)getResponseAsString(191-197)
.vortex/installer/src/Prompts/PromptManager.php (3)
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (1)
VersionScheme(11-117).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19)
🪛 LanguageTool
.vortex/docs/content/workflows/variables.mdx
[style] ~1987-~1987: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... Versioning scheme used for releases. Can be one of: calver, semver, other
@s...
(MISSING_IT_THERE)
docs/releasing.md
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...release/X.Y.Zin GitHub repository. 2. Release tag exists asX.Y.Z` in GitHub reposit...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
.vortex/docs/content/workflows/releasing.mdx
[style] ~37-~37: To form a complete sentence, be sure to include a subject.
Context: ...est development code, not yet released. May be unstable, but CI tests should pass. ...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.18.1)
docs/releasing.md
9-9: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- 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 (13)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
36-36: LGTM! VersionScheme integration in test base class is consistent.The import, expected defaults (CALVER), and TUI answer mappings correctly integrate the new VersionScheme handler into the test infrastructure.
Also applies to: 134-134, 207-207
.env (1)
219-227: LGTM! Clean addition of VORTEX_RELEASE_VERSION_SCHEME variable.The new section is well-documented with clear allowed values (calver, semver, other), a sensible default (calver), and a helpful reference link.
.vortex/docs/cspell.json (1)
14-15: LGTM! Spell-check dictionary appropriately expanded.The additions (Runsheet/runsheet variants, calver, hotfixes) align with the versioning and release workflow terminology introduced in this PR.
Also applies to: 22-22, 40-40, 66-67
.vortex/installer/src/Prompts/Handlers/VersionScheme.php (4)
11-24: LGTM! Clean class structure with clear constants.The handler constants (CALVER, SEMVER, OTHER) and label are well-defined and consistent.
29-54: LGTM! Clear and informative prompt description.The description provides good context for each versioning option with examples and reference links. The CalVer example "24.1.0" could optionally be updated to "25.1.0" to reflect the current year, but this is purely cosmetic.
59-92: LGTM! Robust implementation of handler methods.The discovery logic correctly validates against known values and safely falls back to NULL for invalid inputs. The default (CALVER) is sensible.
97-115: LGTM! Token removal logic is correct.The process() method correctly:
- Persists the selection to
.env- Removes appropriate documentation tokens based on the chosen scheme
- Handles the "Other" case by removing both CalVer and SemVer-specific content
.vortex/installer/src/Prompts/PromptManager.php (4)
40-40: LGTM! VersionScheme correctly imported and response count updated.The import and TOTAL_RESPONSES increment (28→29) are consistent with adding a new top-level prompt.
Also applies to: 69-69
158-158: LGTM! VersionScheme prompt correctly integrated into prompt flow.The placement in the "Code repository" section after CodeProvider is logical, and the select() prompt correctly uses the handler's configuration.
292-292: LGTM! VersionScheme processor correctly ordered.Placing VersionScheme before CodeProvider in the processors list correctly follows the reverse-order processing pattern documented in the method.
394-394: LGTM! Version scheme added to response summary.The summary entry correctly displays the selected version scheme value.
.vortex/installer/tests/Unit/Handlers/VersionSchemePromptManagerTest.php (1)
15-60: LGTM! Good test coverage with room for enhancement.The test scenarios effectively cover the main paths: prompting, discovery of valid values, and missing configuration.
Consider adding a test case for invalid
.envvalues to verify the guard inVersionScheme::discover():'version scheme - discovery - invalid value' => [ [], [VersionScheme::id() => VersionScheme::CALVER] + $expected_installed, function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); $test->stubDotenvValue('VORTEX_RELEASE_VERSION_SCHEME', 'invalid'); }, ],This would verify that invalid values correctly fall back to the CALVER default.
.vortex/installer/tests/Functional/Handlers/VersionSchemeInstallTest.php (1)
7-52: Disregard the import concern;AbstractInstallTestCaseis in the same namespace and resolves correctly.Verification confirms that
AbstractInstallTestCaseis defined in.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.phpwithin the same namespace (DrevOps\VortexInstaller\Tests\Functional\Handlers) asVersionSchemeInstallTest. In PHP, when a class extends another class in the same namespace, an explicitusestatement is not required—namespace resolution handles it automatically. This is the standard pattern used throughout the test suite; all other test files in that directory follow the same approach without issues.The test logic itself is sound: the data provider correctly seeds environment variables and validates that
VersionSchemeproduces expected output for each scheme option.Likely an incorrect or invalid review comment.
|
|
||
| - name: Generate CalVer version | ||
| if: vars.VORTEX_RELEASE_VERSION_SCHEME == 'calver' | ||
| if: ${{ contains(env.VORTEX_RELEASE_VERSION_SCHEME, 'calver') }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using exact equality instead of contains for CalVer check.
The contains() check will match substrings like "notcalver" or "calver_test". Since the variable is controlled and has documented values (calver, semver, other), an exact equality check would be more precise:
if: ${{ env.VORTEX_RELEASE_VERSION_SCHEME == 'calver' }}🤖 Prompt for AI Agents
.github/workflows/draft-release-notes.yml around line 26: the workflow uses
contains(env.VORTEX_RELEASE_VERSION_SCHEME, 'calver') which can match unintended
substrings; change the condition to use an exact equality comparison against the
variable (env.VORTEX_RELEASE_VERSION_SCHEME == 'calver') so the step only runs
when the value is exactly "calver".
Closes #2044
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.