-
-
Notifications
You must be signed in to change notification settings - Fork 0
Updated to Vortex 25.6.1 #65
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
📝 WalkthroughWalkthroughThis update synchronizes service image versions and workflow cache keys to the latest releases, standardizes environment variable naming from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Ahoy
participant Scripts
participant Docker
participant Environment
User->>Ahoy: Run test-bdd or test-behat
Ahoy->>Scripts: Execute Behat tests
User->>Docker: Start containers (using updated images)
Docker->>Environment: Use new image versions and environment variables
User->>Scripts: Run deployment or utility scripts
Scripts->>Environment: Validate env vars (using fail/note)
Scripts->>Environment: Optionally login to registry (credentials optional)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (22)
.ahoy.yml(1 hunks).docker/cli.dockerfile(1 hunks).docker/database.dockerfile(1 hunks).docker/nginx-drupal.dockerfile(1 hunks).docker/php.dockerfile(1 hunks).env(3 hunks).github/workflows/build-test-deploy.yml(4 hunks)CLAUDE.md(1 hunks)README.md(1 hunks)docker-compose.yml(3 hunks)scripts/vortex/deploy-artifact.sh(1 hunks)scripts/vortex/deploy-container-registry.sh(1 hunks)scripts/vortex/deploy.sh(1 hunks)scripts/vortex/doctor.sh(1 hunks)scripts/vortex/download-db-url.sh(1 hunks)scripts/vortex/login-container-registry.sh(2 hunks)scripts/vortex/reset.sh(1 hunks)scripts/vortex/task-custom-lagoon.sh(1 hunks)tests/behat/features/homepage.feature(1 hunks)tests/behat/features/login.feature(1 hunks)web/sites/default/includes/modules/settings.redis.php(1 hunks)web/sites/default/includes/modules/settings.system.php(1 hunks)
🔇 Additional comments (25)
scripts/vortex/reset.sh (1)
55-55: LGTM: grammar correction only
This change fixes a stray comma in the task message and does not alter any logic.scripts/vortex/deploy-artifact.sh (1)
67-73: Centralize error handling usingfail.This change correctly replaces raw
echocalls with the standardizedfailhelper for consistency in error reporting across scripts.scripts/vortex/download-db-url.sh (1)
43-43: Standardize error output withfail.Using
failfor missingVORTEX_DB_DOWNLOAD_URLaligns with other scripts' error handling conventions.scripts/vortex/doctor.sh (1)
129-130: Usenotefor follow-up instructions.Switching from plain
echoto thenotehelper ensures consistent informational message formatting after container failure logging.scripts/vortex/deploy.sh (1)
64-68: Relocate and format skip flag logic.Moving the
$VORTEX_DEPLOY_SKIPcheck belowinfoand usingnote/passhelpers standardizes the output and improves script readability..env (3)
45-48: ClarifyDRUPAL_CONFIG_PATHusage.The added comments clearly describe the path’s relation to the web root and fallback behavior, improving developer understanding.
68-68: Rename Valkey flag to Redis integration.Renaming
DRUPAL_VALKEY_ENABLEDtoDRUPAL_REDIS_ENABLEDunifies feature toggles. Verify that all code, Docker Compose, and settings files reference this updated variable.
130-130: Remove stray comment marker.The formatting fix removes the extra
#before theahoy drush sql:connectcomment, improving comment readability.scripts/vortex/task-custom-lagoon.sh (1)
66-69: LGTM! Improved error reporting consistency.The standardization to use the
failfunction for error reporting improves consistency and provides better formatted output with color coding. The logic flow remains correct with proper exit handling.README.md (1)
15-15: Version update looks correct.The Vortex badge version update from 25.6.0 to 25.6.1 aligns with the PR objectives.
tests/behat/features/login.feature (1)
4-6: Improved feature description clarity.The change from "site owner" to "site administrator" and the more specific purpose description better aligns with the actual test scenarios and improves the feature's clarity.
web/sites/default/includes/modules/settings.system.php (1)
11-11: Good type safety improvement.Adding explicit integer casting ensures the environment variable value is properly typed for Drupal configuration, preventing potential type-related issues.
.ahoy.yml (1)
231-231: Useful alias addition.Adding
test-behatas an alias fortest-bddprovides more intuitive command naming while maintaining backward compatibility.web/sites/default/includes/modules/settings.redis.php (1)
16-24: Comments updated to reference DRUPAL_REDIS_ENABLED
The comment block has been correctly updated to use the newDRUPAL_REDIS_ENABLEDvariable. Ensure that this rename is also reflected in your project’s.envdocumentation and deployment guides to maintain consistency..github/workflows/build-test-deploy.yml (3)
115-119: Bumped cache key prefix for DB restore
The cache key and restore-keys in the database job have been updated fromv25.5.0tov25.6.1. This change is consistent and looks good.
146-147: Bumped cache key prefix for DB save
The save cache key for the database job correctly matches the new version prefixv25.6.1. No further action needed.
213-214: Bumped cache key prefix for build restore
The build job’srestore-keysentry is now correctly usingv25.6.1. All cache key changes in this workflow are aligned.tests/behat/features/homepage.feature (1)
4-6: Refined user role in feature description
Changing from “site owner” to “site visitor” clarifies the user’s intent. Verify that this role label is consistent across all BDD scenarios.scripts/vortex/deploy-container-registry.sh (1)
61-61: Standardized output withnotehelper
Replacing a plainechowith thenotefunction aligns this script’s output format with the rest of the Vortex tooling.docker-compose.yml (3)
71-71: Renamed environment flag to DRUPAL_REDIS_ENABLED
TheDRUPAL_REDIS_ENABLEDvariable now replaces the oldDRUPAL_VALKEY_ENABLED. Confirm that your.envfile and any deployment documentation are updated accordingly.
146-146: Updated default database image version
Bumped the default MySQL image from25.5.0to25.6.0. This aligns with the overall version bump strategy.
156-156: Updated Valkey service image version
The Valkey container tag has been correctly updated to25.6.0. No additional changes required here.CLAUDE.md (2)
250-256: Verify External Resource URL
The Vortex link is listed ashttps://www.vortextemplate.com. Please confirm the correct domain and update if necessary.
120-130: Inconsistent with Env Var Rename
The PR summary indicates a rename fromDRUPAL_VALKEY_ENABLEDtoDRUPAL_REDIS_ENABLED, but I don’t see this reflected under Configuration Management or.envguidance. Please document the new variable name and update examples accordingly.Likely an incorrect or invalid review comment.
scripts/vortex/login-container-registry.sh (1)
25-32: Graceful Defaults Are Good, But Validate Credentials
DefaultingVORTEX_CONTAINER_REGISTRY_USERandVORTEX_CONTAINER_REGISTRY_PASSto empty strings allows skipping login, but it may silently hide misconfiguration. Consider rejecting whitespace-only values or logging a debug warning when credentials are unset.
| FROM ${CLI_IMAGE:-cli} AS cli | ||
|
|
||
| FROM uselagoon/nginx-drupal:25.5.0 | ||
| FROM uselagoon/nginx-drupal:25.6.0 |
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.
Critical: version bump mismatch
The PR is titled "Updated to Vortex 25.6.1", but this Dockerfile still pins uselagoon/nginx-drupal at 25.6.0. Please align the base image tag with the intended 25.6.1 release.
-FROM uselagoon/nginx-drupal:25.6.0
+FROM uselagoon/nginx-drupal:25.6.1📝 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.
| FROM uselagoon/nginx-drupal:25.6.0 | |
| FROM uselagoon/nginx-drupal:25.6.1 |
🤖 Prompt for AI Agents
In .docker/nginx-drupal.dockerfile at line 14, the base image version is pinned
to 25.6.0, which does not match the PR title indicating an update to version
25.6.1. Update the FROM statement to use uselagoon/nginx-drupal:25.6.1 to align
the Dockerfile with the intended version bump.
| # @see https://github.com/uselagoon/lagoon-images/tree/main/images/php-cli-drupal | ||
|
|
||
| FROM uselagoon/php-8.3-cli-drupal:25.5.0 | ||
| FROM uselagoon/php-8.3-cli-drupal:25.6.0 |
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.
Critical: version bump mismatch
This Dockerfile updates uselagoon/php-8.3-cli-drupal to 25.6.0, but the PR targets Vortex 25.6.1. Please bump to 25.6.1 for consistency across images.
-FROM uselagoon/php-8.3-cli-drupal:25.6.0
+FROM uselagoon/php-8.3-cli-drupal:25.6.1📝 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.
| FROM uselagoon/php-8.3-cli-drupal:25.6.0 | |
| -FROM uselagoon/php-8.3-cli-drupal:25.6.0 | |
| +FROM uselagoon/php-8.3-cli-drupal:25.6.1 |
🤖 Prompt for AI Agents
In .docker/cli.dockerfile at line 10, the base image version is set to 25.6.0,
but the PR targets version 25.6.1. Update the version tag in the FROM statement
from 25.6.0 to 25.6.1 to ensure consistency across images.
| # The ARG value will be updated with a value passed from docker-compose.yml | ||
|
|
||
| ARG IMAGE=uselagoon/mysql-8.4:25.5.0 | ||
| ARG IMAGE=uselagoon/mysql-8.4:25.6.0 |
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.
Critical: version bump mismatch
The database image is set to 25.6.0, but the overarching update is for Vortex 25.6.1. Please update this ARG to 25.6.1.
-ARG IMAGE=uselagoon/mysql-8.4:25.6.0
+ARG IMAGE=uselagoon/mysql-8.4:25.6.1📝 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.
| ARG IMAGE=uselagoon/mysql-8.4:25.6.0 | |
| ARG IMAGE=uselagoon/mysql-8.4:25.6.1 |
🤖 Prompt for AI Agents
In .docker/database.dockerfile at line 8, the ARG IMAGE version is set to
25.6.0, which does not match the Vortex update version 25.6.1. Update the
version number in the ARG IMAGE declaration from 25.6.0 to 25.6.1 to ensure
consistency with the overall version bump.
| FROM ${CLI_IMAGE:-cli} AS cli | ||
|
|
||
| FROM uselagoon/php-8.3-fpm:25.5.0 | ||
| FROM uselagoon/php-8.3-fpm:25.6.0 |
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.
Critical: version bump mismatch
This Dockerfile still uses uselagoon/php-8.3-fpm:25.6.0, whereas the PR updates should target Vortex 25.6.1. Please bump the tag accordingly.
-FROM uselagoon/php-8.3-fpm:25.6.0
+FROM uselagoon/php-8.3-fpm:25.6.1📝 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.
| FROM uselagoon/php-8.3-fpm:25.6.0 | |
| FROM uselagoon/php-8.3-fpm:25.6.1 |
🤖 Prompt for AI Agents
In .docker/php.dockerfile at line 15, update the Docker image tag from
uselagoon/php-8.3-fpm:25.6.0 to uselagoon/php-8.3-fpm:25.6.1 to match the
intended version bump for Vortex 25.6.1.
| - name: Show cache key for database caching | ||
| run: echo 'v25.5.0-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}' | ||
| run: echo 'v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}' |
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.
Shell quoting error in echo command
The run: echo 'v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-…' line uses single quotes nested inside single quotes, which will break. Use double quotes around the outer string or escape the inner ones.
Apply this diff:
- run: echo 'v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}'
+ run: echo "v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}"📝 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.
| run: echo 'v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}' | |
| run: echo "v25.6.1-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}" |
🤖 Prompt for AI Agents
In .github/workflows/build-test-deploy.yml at line 200, the echo command uses
single quotes around a string that contains nested single quotes, causing a
shell quoting error. Fix this by replacing the outer single quotes with double
quotes or by escaping the inner single quotes to ensure the command runs
correctly.
| #### Behat Feature Writing Standards | ||
|
|
||
| When creating or updating Behat feature files, follow these conventions: | ||
|
|
||
| - **User Story Format**: All features must use the standard user story format: | ||
| ```gherkin | ||
| As a [user type] | ||
| I want to [action] | ||
| So that [benefit] | ||
| ``` |
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 (assertive)
Link to Official Behat Docs
The “Behat Feature Writing Standards” section is thorough, but it would be helpful to include a hyperlink to the official Behat documentation or an internal example repository for quick reference.
🤖 Prompt for AI Agents
In CLAUDE.md around lines 71 to 80, add a hyperlink to the official Behat
documentation or an internal example repository within the "Behat Feature
Writing Standards" section. This will provide quick reference for users and
improve the usefulness of the documentation. Insert the link near the section
title or introductory text for clear visibility.
| # Vortex Drupal Project - Development Guide | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
| ## Project Overview | ||
|
|
||
| # DrevOps Website Developer Guidelines | ||
| This is a Drupal project built with **Vortex** - a comprehensive Drupal project template by DrevOps that provides production-ready development and deployment workflows. |
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 (assertive)
Missing Table of Contents
The guide is extensive; adding a TOC at the top (e.g., generated via markdown-toc) would greatly improve navigation for new contributors.
🤖 Prompt for AI Agents
In CLAUDE.md at the beginning of the file (lines 1 to 5), add a Table of
Contents (TOC) after the initial project overview section. Generate the TOC
using a tool like markdown-toc to automatically list all major headings in the
document. Insert the TOC in markdown format so that it provides clickable links
to each section, improving navigation for new contributors.
| # Path to Docker configuration directory. | ||
| DOCKER_CONFIG="${DOCKER_CONFIG:-${HOME}/.docker}" | ||
|
|
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 (assertive)
Ensure Docker Config Directory Exists
You default DOCKER_CONFIG to ${HOME}/.docker, but if the directory doesn’t exist docker login will fail. Add a check or auto-create the directory before login.
🤖 Prompt for AI Agents
In scripts/vortex/login-container-registry.sh around lines 34 to 36, the script
sets DOCKER_CONFIG to ${HOME}/.docker by default but does not ensure this
directory exists, which can cause docker login to fail. Add a check to verify if
the DOCKER_CONFIG directory exists, and if not, create it using mkdir -p before
running docker login.
| note "Skipping login to the container registry as either VORTEX_CONTAINER_REGISTRY_USER or VORTEX_CONTAINER_REGISTRY_PASS was not provided." | ||
| fi |
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 (assertive)
Differentiate Missing User vs Pass
The skip message lumps both missing user or pass together. Splitting it to indicate which variable is unset can aid troubleshooting:
if [ -z "$VORTEX_CONTAINER_REGISTRY_USER" ]; then note "Skipping login: user is unset."
elif [ -z "$VORTEX_CONTAINER_REGISTRY_PASS" ]; then note "Skipping login: password is unset."
fi🤖 Prompt for AI Agents
In scripts/vortex/login-container-registry.sh at lines 61-62, the current skip
message does not specify whether the user or password environment variable is
missing. Modify the conditional checks to separately detect if
VORTEX_CONTAINER_REGISTRY_USER or VORTEX_CONTAINER_REGISTRY_PASS is unset, and
output distinct messages for each case to improve clarity during
troubleshooting.
| [ -z "$(echo "${VORTEX_CONTAINER_REGISTRY}" | xargs)" ] && fail "VORTEX_CONTAINER_REGISTRY should not be empty." && exit 1 | ||
|
|
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 (assertive)
Optimize Whitespace Trimming
Using echo … | xargs works, but you could simplify trimming with Bash parameter expansion:
VORTEX_CONTAINER_REGISTRY="${VORTEX_CONTAINER_REGISTRY//[[:space:]]/}"This avoids spawning a subshell.
🤖 Prompt for AI Agents
In scripts/vortex/login-container-registry.sh around lines 53 to 54, the current
check trims whitespace using echo piped to xargs, which spawns a subshell
unnecessarily. Replace this with Bash parameter expansion to remove all
whitespace from VORTEX_CONTAINER_REGISTRY directly using the pattern
substitution syntax, then check if the resulting variable is empty to decide
failure, improving efficiency by avoiding external commands.
ba482f4 to
5da715c
Compare
5da715c to
6d2285e
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: 6
♻️ Duplicate comments (7)
.docker/database.dockerfile (1)
8-8: Critical: version bump mismatch
TheARG IMAGEis set to:25.6.0, but this PR updates Vortex to 25.6.1. Please bump the tag to maintain consistency across images.-ARG IMAGE=uselagoon/mysql-8.4:25.6.0 +ARG IMAGE=uselagoon/mysql-8.4:25.6.1.docker/cli.dockerfile (1)
10-10: Critical: version bump mismatch
The base image tag is still25.6.0, whereas the PR targets Vortex 25.6.1. Update the version accordingly:-FROM uselagoon/php-8.3-cli-drupal:25.6.0 +FROM uselagoon/php-8.3-cli-drupal:25.6.1.docker/php.dockerfile (1)
15-15: Critical: version bump mismatch
ThisFROMline uses25.6.0, but the PR should align with Vortex 25.6.1. Please bump the tag:-FROM uselagoon/php-8.3-fpm:25.6.0 +FROM uselagoon/php-8.3-fpm:25.6.1.docker/nginx-drupal.dockerfile (1)
14-14: Critical: version bump mismatch
The PR targets Vortex 25.6.1, but the base image is still pinned at 25.6.0.-FROM uselagoon/nginx-drupal:25.6.0 +FROM uselagoon/nginx-drupal:25.6.1CLAUDE.md (3)
1-5: Missing Table of Contents
This guide is extensive—adding a clickable TOC right after the main title will help readers navigate quickly.
71-80: Link to Official Behat Docs
Include a hyperlink to the official Behat documentation (or an internal example repo) in the “Behat Feature Writing Standards” section to provide quick reference.
171-178: Clarify Lagoon Setup
The CI/CD section mentions Lagoon but omits authentication/setup steps. Consider linking to internal docs on configuring Lagoon credentials.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (23)
.ahoy.yml(1 hunks).docker/cli.dockerfile(1 hunks).docker/database.dockerfile(1 hunks).docker/nginx-drupal.dockerfile(1 hunks).docker/php.dockerfile(1 hunks).env(3 hunks).github/workflows/build-test-deploy.yml(4 hunks)CLAUDE.md(1 hunks)README.md(1 hunks)docker-compose.yml(3 hunks)scripts/vortex/deploy-artifact.sh(1 hunks)scripts/vortex/deploy-container-registry.sh(1 hunks)scripts/vortex/deploy.sh(1 hunks)scripts/vortex/doctor.sh(1 hunks)scripts/vortex/download-db-url.sh(1 hunks)scripts/vortex/login-container-registry.sh(2 hunks)scripts/vortex/reset.sh(1 hunks)scripts/vortex/task-custom-lagoon.sh(1 hunks)tests/behat/features/homepage.feature(1 hunks)tests/behat/features/login.feature(1 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(3 hunks)web/sites/default/includes/modules/settings.redis.php(2 hunks)web/sites/default/includes/modules/settings.system.php(1 hunks)
🔇 Additional comments (26)
scripts/vortex/download-db-url.sh (1)
43-43: Standardize error reporting usingfail
Switching from a rawechoto the sharedfailhelper ensures consistent error formatting across all Vortex scripts.scripts/vortex/reset.sh (1)
55-55: Approve: typo fix in task message
The extraneous comma has been removed, improving readability and consistency with other task messages.scripts/vortex/deploy-artifact.sh (1)
67-73: Approve: standardized error reporting
Replaced rawechocalls with the centralizedfailfunction for missing variables, ensuring consistent formatting across scripts. Logic and messaging look correct..ahoy.yml (1)
229-233: Addtest-behatalias for BDD tests
The new alias lets users invoke the existingtest-bddcommand astest-behatwithout changing behavior.scripts/vortex/doctor.sh (1)
127-131: Standardize messaging withnotefunction
Replacing rawechowithnotecalls ensures consistent formatted output across scripts..env (2)
45-49: ClarifyDRUPAL_CONFIG_PATHdocumentation
The added comments explain the fallback behavior and expected path format, improving maintainability.
66-69: RenameDRUPAL_VALKEY_ENABLEDtoDRUPAL_REDIS_ENABLED
Consistent Redis naming aligns with updated module usage. Verify no residualDRUPAL_VALKEY_ENABLEDreferences:rg -F 'DRUPAL_VALKEY_ENABLED'web/sites/default/includes/modules/settings.redis.php (4)
5-10: Update docblock to reflect Redis terminology
The revised comments correctly describe usingDRUPAL_REDIS_environment variables for both Redis and Valkey services.
20-27: DocumentDRUPAL_REDIS_ENABLEDusage
This multi-step comment clarifies the deployment strategy and the two-phase rollout of the flag.
34-35: RetainVALKEY_prefixes for backward compatibility
The fallback order (VALKEY_thenREDIS_) preserves existing deployments while supporting new variables.
40-43: VerifyVORTEX_REDIS_EXTENSION_LOADEDlogic
The conditional handles both default extension checks and explicit overrides. Confirm that tests cover both branches.tests/behat/features/login.feature (1)
4-6: LGTM! User story clarification improves test accuracy.The update from "site owner" to "site administrator" better aligns with the test scenarios that specifically test Administrator role login. The refined purpose statement is also more precise about the intended functionality being tested.
web/sites/default/includes/modules/settings.system.php (1)
11-11: LGTM! Explicit integer casting improves type safety.Adding
(int)casting ensures the cache max age configuration is always an integer, preventing potential type-related issues when the environment variable is set as a string.README.md (2)
1-2: LGTM! Proper HTML structure for centered content.The opening
<div align="center">tag correctly pairs with the existing closing tag at line 15 to center the badge section.
13-13: LGTM! Version update aligns with PR objectives.The Vortex badge version update from 25.6.0 to 25.6.1 correctly reflects the PR objective of updating to Vortex 25.6.1. Both the badge image URL and hyperlink target are consistently updated.
scripts/vortex/deploy-container-registry.sh (1)
61-61: LGTM! Standardized output formatting improves consistency.Replacing the direct
echostatement with thenote()function call provides consistent formatting and colorization that aligns with other output statements in the script.tests/behat/features/homepage.feature (1)
4-6: LGTM! User story better represents homepage testing context.The update from "site owner" to "site visitor" more accurately reflects the anonymous user scenarios being tested. The refined purpose statement appropriately describes typical homepage usage patterns.
.github/workflows/build-test-deploy.yml (1)
115-115: Cache key version updates look good.The version prefix updates from
v25.5.0tov25.6.1are consistently applied across all cache operations, ensuring proper cache invalidation for the new version.Also applies to: 119-119, 146-146, 212-212, 214-214
scripts/vortex/deploy.sh (1)
64-68: Improved deployment skip logic placement and output formatting.Moving the skip check after the "Started deployment." message provides better user feedback, and using the standardized
note()andpass()functions improves output consistency across the deployment tooling.docker-compose.yml (2)
71-71: Environment variable rename aligns with Redis standardization.The change from
DRUPAL_VALKEY_ENABLEDtoDRUPAL_REDIS_ENABLEDis consistent with the broader Redis/Valkey terminology standardization across the codebase.
146-146: Service image version updates are consistent.The updates from
25.5.0to25.6.0for both database and Valkey services align with the overall Vortex version update.Also applies to: 156-156
scripts/vortex/login-container-registry.sh (2)
25-27: Good improvement making container registry credentials optional.The updated documentation clearly indicates that missing credentials will result in skipping the login step, making the script more flexible for different deployment scenarios.
Also applies to: 30-33
34-35: New DOCKER_CONFIG variable improves flexibility.The introduction of the configurable Docker configuration directory path enhances the script's adaptability to different environments.
tests/phpunit/Drupal/SwitchableSettingsTest.php (2)
235-235: Environment variable updates align with codebase changes.The updates from
DRUPAL_VALKEY_ENABLEDtoDRUPAL_REDIS_ENABLEDandVORTEX_VALKEY_EXTENSION_LOADEDtoVORTEX_REDIS_EXTENSION_LOADEDare consistent with the environment variable standardization across the project.Also applies to: 238-238, 259-259, 262-262
278-327: Excellent test coverage for Redis variable handling.The new test methods provide comprehensive coverage for:
- Redis-specific environment variables (
testRedisVariables)- Precedence logic when both VALKEY_* and REDIS_* variables are present (
testRedisValkeyPrecedence)The tests properly handle the
bootstrap_container_definitioncleanup and verify the correct Redis connection settings and cache backend configuration.CLAUDE.md (1)
65-69: Inconsistent alias for BDD tests
The PR summary and .ahoy.yml reference atest-behatalias, but the docs showahoy test-bdd. Please verify which alias is correct and align the docs accordingly.Likely an incorrect or invalid review comment.
| [ -z "${VORTEX_TASK_LAGOON_NAME}" ] && fail "Missing required value for VORTEX_TASK_LAGOON_NAME." && exit 1 | ||
| [ -z "${VORTEX_TASK_LAGOON_BRANCH}" ] && fail "Missing required value for VORTEX_TASK_LAGOON_BRANCH." && exit 1 | ||
| [ -z "${VORTEX_TASK_LAGOON_COMMAND}" ] && fail "Missing required value for VORTEX_TASK_LAGOON_COMMAND." && exit 1 | ||
| [ -z "${VORTEX_TASK_LAGOON_PROJECT}" ] && fail "Missing required value for VORTEX_TASK_LAGOON_PROJECT." && exit 1 |
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 (assertive)
DRY up environment variable validation
These repeated -z … && fail … && exit 1 checks can be consolidated into a loop to reduce duplication and ease future maintenance. For example:
required_vars=(
VORTEX_TASK_LAGOON_NAME
VORTEX_TASK_LAGOON_BRANCH
VORTEX_TASK_LAGOON_COMMAND
VORTEX_TASK_LAGOON_PROJECT
)
for var in "${required_vars[@]}"; do
if [ -z "${!var}" ]; then
fail "Missing required value for ${var}."
exit 1
fi
done🤖 Prompt for AI Agents
In scripts/vortex/task-custom-lagoon.sh around lines 66 to 69, the environment
variable validation is repeated for each variable. Refactor this by creating an
array of required variable names and loop over it, checking if each variable is
empty. If any variable is missing, call fail with the appropriate message and
exit with status 1. This will reduce code duplication and improve
maintainability.
| - **Vortex Documentation**: https://www.vortextemplate.com | ||
| - **Drupal Documentation**: https://www.drupal.org/docs | ||
| - **Drush Documentation**: https://www.drush.org |
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 (assertive)
Use authoritative Vortex docs link
Update the “Vortex Documentation” URL to the official GitHub repo (https://github.com/drevops/vortex) instead of the placeholder site.
🤖 Prompt for AI Agents
In CLAUDE.md around lines 252 to 254, the "Vortex Documentation" URL currently
points to a placeholder site. Update this URL to the official GitHub repository
at https://github.com/drevops/vortex to ensure the documentation link is
authoritative and accurate.
|
|
||
| ``` | ||
| ├── config/ # Drupal configuration (exported) | ||
| │ ├── default/ # Default configuration | ||
| │ ├── dev/ # Development-specific config | ||
| │ ├── stage/ # Staging-specific config | ||
| │ └── ci/ # CI-specific config | ||
| ├── scripts/ | ||
| │ ├── vortex/ # Core Vortex deployment scripts | ||
| │ └── custom/ # Project-specific custom scripts | ||
| ├── web/ # Drupal webroot | ||
| │ ├── modules/custom/ # Custom modules | ||
| │ ├── themes/custom/ # Custom themes | ||
| │ └── sites/default/ # Drupal site configuration | ||
| ├── tests/ | ||
| │ ├── behat/ # Behavioral tests (BDD) | ||
| │ └── phpunit/ # Unit/integration tests | ||
| ├── docker-compose.yml # Local development environment | ||
| └── .env # Environment configuration | ||
| ``` |
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 (assertive)
Project Structure code block formatting
The file tree isn’t wrapped in a fenced code block. Add triple backticks around the diagram so it renders properly.
🤖 Prompt for AI Agents
In CLAUDE.md around lines 99 to 118, the project structure diagram is not
enclosed in a fenced code block, causing formatting issues. Wrap the entire file
tree diagram with triple backticks (```) before and after the block to ensure it
renders correctly as a code block in markdown.
| ### Environment management | ||
|
|
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 (assertive)
Consistent Heading Capitalization
Section titles mix title case and sentence case (e.g., “Environment management” vs. “Code Quality”). Standardize to Title Case for all headings.
Also applies to: 32-33, 39-39, 48-49, 56-56
🤖 Prompt for AI Agents
In CLAUDE.md around lines 22 to 23 and also at lines 32-33, 39, 48-49, and 56,
the section headings use inconsistent capitalization styles. Change all these
headings to use Title Case capitalization consistently, capitalizing the first
letter of each major word in the headings.
| ## Local Development Commands (Ahoy) | ||
|
|
||
| ### Environment Management | ||
| - `ahoy build` - Full build: reset, start containers, install dependencies, provision site | ||
| - `ahoy up` - Start containers (with optional `--build --force-recreate`) | ||
| - `ahoy down` - Stop and remove containers (removes database!) | ||
| - `ahoy provision` - Re-import database and run update/cache/deploy commands | ||
| - `ahoy reset [hard]` - Reset environment (soft) or to last commit (hard) | ||
| ### Environment management | ||
|
|
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.
🛠️ Refactor suggestion
Add an Environment Variables section
We need a dedicated section listing key env vars (e.g., DRUPAL_REDIS_ENABLED, previously DRUPAL_VALKEY_ENABLED) so developers know how to toggle Redis and other services.
🤖 Prompt for AI Agents
In CLAUDE.md around lines 20 to 23, add a new section titled "Environment
Variables" under the "Local Development Commands (Ahoy)" heading. This section
should list key environment variables such as DRUPAL_REDIS_ENABLED and
DRUPAL_VALKEY_ENABLED, with brief descriptions explaining their purpose and how
to use them to toggle Redis and other services during local development.
| ## Services Integration | ||
|
|
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.
Empty Services Integration section
The “Services Integration” heading is empty. Either populate it with instructions (e.g., Redis setup, search API) or remove it.
🤖 Prompt for AI Agents
In CLAUDE.md around lines 166 to 167, the "Services Integration" section is
currently empty. To fix this, either add relevant content such as setup
instructions for services like Redis or the search API, or remove the "Services
Integration" heading entirely if no content is needed.
Updated to https://github.com/drevops/vortex/releases/tag/25.6.1
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation
Refactor