-
-
Notifications
You must be signed in to change notification settings - Fork 0
Updated to the latest Vortex. #69
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
📝 WalkthroughWalkthroughThe changes update Dockerfiles for ClamAV, Solr, and the CLI to improve command formatting and conditional logic, particularly around theme asset building. The GitHub Actions workflow is adjusted to limit redundant linting in parallel jobs and disable theme builds on scheduled runs. Docker Compose now passes a Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHubActions
participant DockerCompose
participant CLIContainer
Developer->>DockerCompose: docker-compose build
DockerCompose->>CLIContainer: Build with DRUPAL_THEME arg (default empty)
CLIContainer->>CLIContainer: If DRUPAL_THEME is non-empty, run yarn install/build in theme dir
Developer->>GitHubActions: Push or schedule workflow
GitHubActions->>GitHubActions: If scheduled, set DRUPAL_THEME=""
GitHubActions->>GitHubActions: For matrix jobs, run lint/validation only on instance 0
🪧 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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (5)
.docker/clamav.dockerfile(1 hunks).docker/cli.dockerfile(3 hunks).docker/solr.dockerfile(1 hunks).github/workflows/build-test-deploy.yml(3 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
.docker/solr.dockerfile (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
.github/workflows/build-test-deploy.yml (2)
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
.docker/cli.dockerfile (4)
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
🔇 Additional comments (3)
.docker/clamav.dockerfile (1)
24-25: Formatting change looks goodSplitting the
RUNcommand across two physical lines keeps the same single-layer semantics while improving readability. No functional or build-size impact detected..docker/solr.dockerfile (1)
20-22: Refactored RUN instruction is fineThe consolidated
sedchain preserves behaviour and keeps it to one layer. No issues spotted.docker-compose.yml (1)
89-89: Build arg passthrough LGTMAdding
DRUPAL_THEMEto the CLI build args keeps compose and Dockerfile in sync. ✅
| # Add more tools. | ||
| RUN apk add --no-cache ncurses pv tzdata autoconf g++ make \ | ||
| && pecl install pcov \ | ||
| && docker-php-ext-enable pcov \ | ||
| && docker-php-ext-install pcntl \ | ||
| && apk del g++ make autoconf | ||
| RUN apk add --no-cache ncurses pv tzdata autoconf g++ make && \ | ||
| pecl install pcov && \ | ||
| docker-php-ext-enable pcov && \ | ||
| docker-php-ext-install pcntl && \ | ||
| apk del g++ make autoconf |
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)
Shrink image by using a virtual build package group
You can avoid leaving behind g++, make, and autoconf layers by installing them as a temporary virtual package and deleting it in one shot:
-RUN apk add --no-cache ncurses pv tzdata autoconf g++ make && \
- pecl install pcov && \
- docker-php-ext-enable pcov && \
- docker-php-ext-install pcntl && \
- apk del g++ make autoconf
+RUN apk add --no-cache ncurses pv tzdata --virtual .build-deps g++ make autoconf && \
+ pecl install pcov && \
+ docker-php-ext-enable pcov && \
+ docker-php-ext-install pcntl && \
+ apk del .build-deps📝 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.
| # Add more tools. | |
| RUN apk add --no-cache ncurses pv tzdata autoconf g++ make \ | |
| && pecl install pcov \ | |
| && docker-php-ext-enable pcov \ | |
| && docker-php-ext-install pcntl \ | |
| && apk del g++ make autoconf | |
| RUN apk add --no-cache ncurses pv tzdata autoconf g++ make && \ | |
| pecl install pcov && \ | |
| docker-php-ext-enable pcov && \ | |
| docker-php-ext-install pcntl && \ | |
| apk del g++ make autoconf | |
| # Add more tools. | |
| RUN apk add --no-cache ncurses pv tzdata --virtual .build-deps g++ make autoconf && \ | |
| pecl install pcov && \ | |
| docker-php-ext-enable pcov && \ | |
| docker-php-ext-install pcntl && \ | |
| apk del .build-deps |
🤖 Prompt for AI Agents
In .docker/cli.dockerfile around lines 49 to 54, the installation of build tools
g++, make, and autoconf leaves residual layers increasing image size. Refactor
the RUN command to install these tools as a virtual package group using apk's
--virtual option, then remove the entire virtual package group in the same RUN
step after use. This ensures all build dependencies are cleaned up in one layer,
shrinking the final image size.
| RUN if [ -n "${DRUPAL_THEME}" ]; then \ | ||
| theme_path="/app/${WEBROOT}/themes/custom/${DRUPAL_THEME}"; \ | ||
| yarn --cwd="${theme_path}" install --frozen-lockfile --no-progress && \ | ||
| yarn --cwd="${theme_path}" run build && \ | ||
| yarn cache clean; \ | ||
| 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.
Guard against missing theme path to prevent build failures
If DRUPAL_THEME is set but the directory is misspelled or absent, the build will exit with a fatal Yarn error. Add an existence check so the image can still be built:
-RUN if [ -n "${DRUPAL_THEME}" ]; then \
- theme_path="/app/${WEBROOT}/themes/custom/${DRUPAL_THEME}"; \
- yarn --cwd="${theme_path}" install --frozen-lockfile --no-progress && \
- yarn --cwd="${theme_path}" run build && \
- yarn cache clean; \
- fi
+RUN if [ -n "${DRUPAL_THEME}" ] && [ -d "/app/${WEBROOT}/themes/custom/${DRUPAL_THEME}" ]; then \
+ yarn --cwd="/app/${WEBROOT}/themes/custom/${DRUPAL_THEME}" install --frozen-lockfile --no-progress && \
+ yarn --cwd="/app/${WEBROOT}/themes/custom/${DRUPAL_THEME}" run build && \
+ yarn cache clean; \
+ else \
+ echo "Skipping theme asset build – DRUPAL_THEME empty or path missing"; \
+ fi🤖 Prompt for AI Agents
In .docker/cli.dockerfile around lines 83 to 88, the current RUN command assumes
the theme directory exists if DRUPAL_THEME is set, causing build failures if the
directory is missing. Modify the script to check if the theme_path directory
actually exists before running yarn commands, so the build does not fail when
the directory is absent or misspelled.
e66d596 to
0e4c032
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.docker/cli.dockerfile (2)
50-54: Reuse prior suggestion: install build tools via virtual package group
83-88: Reuse prior suggestion: guard for missing theme directory to avoid build failure
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (5)
.docker/clamav.dockerfile(1 hunks).docker/cli.dockerfile(3 hunks).docker/solr.dockerfile(1 hunks).github/workflows/build-test-deploy.yml(3 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
.docker/solr.dockerfile (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
.docker/cli.dockerfile (4)
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Docker images are updated for containers via RenovateBot
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Docker images are updated for containers via RenovateBot
.github/workflows/build-test-deploy.yml (2)
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-17T16:32:41.992Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.308Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
🔇 Additional comments (5)
.docker/clamav.dockerfile (1)
24-29: Split RUN chain is clean and functionally sound – LGTMMoving
rm /tmp/clamav.confonto its own line keeps the append/remove flow readable without altering behaviour.docker-compose.yml (1)
89-90: Build arg forwardsDRUPAL_THEMEcorrectlyPassing the variable into the CLI build keeps the Dockerfile and runtime in sync. Looks good.
.github/workflows/build-test-deploy.yml (2)
95-101: Empty-string assignment fixes previous quoting issue – nice catchUsing
DRUPAL_THEME=(no quotes) avoids the literal""value that broke shell checks in the Dockerfile.
221-290: Conditional gating of heavy linters is sensibleRestricting lint/analysis steps to the primary matrix instance and guarding theme lints behind
env.DRUPAL_THEME != ''trims CI time without losing coverage..docker/cli.dockerfile (1)
34-36: Default emptyDRUPAL_THEMEaligns with optional theme builds – LGTM
| RUN sed -i -e "s#<dataDir>\${solr.data.dir:}#<dataDir>/var/solr/\${solr.core.name}#g" /solr-conf/conf/solrconfig.xml && \ | ||
| sed -i -e "s#solr.lock.type:native#solr.lock.type:none#g" /solr-conf/conf/solrconfig.xml && \ | ||
| sed -i -e "s#solr.autoSoftCommit.MaxTime=5000#solr.autoSoftCommit.MaxTime=-1#g" /solr-conf/conf/solrcore.properties |
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)
Minor perf nit – combine the three sed edits
Each sed -i rewrites the file, incurring three I/O passes. You can fuse them into one for a marginally smaller layer:
-RUN sed -i -e "s#<dataDir>\${solr.data.dir:}#<dataDir>/var/solr/\${solr.core.name}#g" /solr-conf/conf/solrconfig.xml && \
- sed -i -e "s#solr.lock.type:native#solr.lock.type:none#g" /solr-conf/conf/solrconfig.xml && \
- sed -i -e "s#solr.autoSoftCommit.MaxTime=5000#solr.autoSoftCommit.MaxTime=-1#g" /solr-conf/conf/solrcore.properties
+RUN sed -i \
+ -e "s#<dataDir>\${solr.data.dir:}#<dataDir>/var/solr/\${solr.core.name}#g" \
+ -e "s#solr.lock.type:native#solr.lock.type:none#g" \
+ -e "s#solr.autoSoftCommit.MaxTime=5000#solr.autoSoftCommit.MaxTime=-1#g" \
+ /solr-conf/conf/solrconfig.xml /solr-conf/conf/solrcore.properties📝 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 sed -i -e "s#<dataDir>\${solr.data.dir:}#<dataDir>/var/solr/\${solr.core.name}#g" /solr-conf/conf/solrconfig.xml && \ | |
| sed -i -e "s#solr.lock.type:native#solr.lock.type:none#g" /solr-conf/conf/solrconfig.xml && \ | |
| sed -i -e "s#solr.autoSoftCommit.MaxTime=5000#solr.autoSoftCommit.MaxTime=-1#g" /solr-conf/conf/solrcore.properties | |
| RUN sed -i \ | |
| -e "s#<dataDir>\${solr.data.dir:}#<dataDir>/var/solr/\${solr.core.name}#g" \ | |
| -e "s#solr.lock.type:native#solr.lock.type:none#g" \ | |
| -e "s#solr.autoSoftCommit.MaxTime=5000#solr.autoSoftCommit.MaxTime=-1#g" \ | |
| /solr-conf/conf/solrconfig.xml /solr-conf/conf/solrcore.properties |
🤖 Prompt for AI Agents
In .docker/solr.dockerfile around lines 20 to 22, the three separate sed
commands each rewrite the file individually, causing unnecessary multiple I/O
operations. Combine all three sed expressions into a single sed command with
multiple -e options to perform all replacements in one pass, reducing the number
of file writes and improving build efficiency.
Summary by CodeRabbit