-
-
Notifications
You must be signed in to change notification settings - Fork 28
Fixed theme CSS output style in Grunt config. #2008
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
WalkthroughRenamed the Sass option key Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2008 +/- ##
========================================
Coverage 78.02% 78.02%
========================================
Files 89 89
Lines 5542 5542
Branches 44 44
========================================
Hits 4324 4324
Misses 1218 1218 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
89f922e to
8e4f7e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/Gruntfile.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/Gruntfile.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/Gruntfile.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/Gruntfile.jsis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (2)
.vortex/tests/bats/_helper.workflow.bash(1 hunks)web/themes/custom/your_site_theme/Gruntfile.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
web/themes/custom/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place all custom Drupal themes under web/themes/custom/[theme_name]/
Files:
web/themes/custom/your_site_theme/Gruntfile.js
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#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/tests/bats/_helper.workflow.bash
⏰ 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). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.vortex/tests/bats/_helper.workflow.bash (1)
773-773: Test expectation correctly aligned with compressed CSS output.Matches the minified Sass output (no space after colon) and keeps consistency with earlier negative assertion for the spaced variant.
| style: 'expanded' | ||
| } |
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.
💡 Verification agent
🧩 Analysis chain
Confirm grunt-sass/sass versions accept style; add fallback for broad compatibility.
Depending on plugin/runtime versions, some stacks only honor outputStyle (legacy render API) while others use style (newer API). Please verify theme package versions to avoid silent reversion to default formatting.
Run to verify versions and other themes’ configs:
If you need cross-version safety, set both keys (harmless for either API):
options: {
implementation: require('sass'),
sourceMap: true,
- style: 'expanded'
+ style: 'expanded',
+ outputStyle: 'expanded'
}
...
options: {
implementation: require('sass'),
sourceMap: false,
- style: 'compressed'
+ style: 'compressed',
+ outputStyle: 'compressed'
}Also applies to: 97-98
🏁 Script executed:
#!/bin/bash
# Inspect theme package versions.
rg -n '"grunt-sass"|\"sass\"' web/themes/custom/**/package.json -C1 || true
# Check all theme Gruntfiles for style keys used.
rg -nP '\b(outputStyle|style)\b' web/themes/custom/**/Gruntfile.js || trueLength of output: 164
🏁 Script executed:
#!/bin/bash
# Inspect Sass-related dependencies and versions in theme package.json files
find web/themes/custom -type f -name package.json -print0 | xargs -0 rg -nP '"(?:grunt-sass|sass|grunt-contrib-sass)"\s*:\s*"[^"]+"' -n || true
# Check all theme Gruntfiles for style and outputStyle usage
rg -nP '\b(outputStyle|style)\b' web/themes/custom/**/Gruntfile.js || trueLength of output: 116
Explicitly set outputStyle alongside style in Gruntfile.js
grunt-sass v4’s task options honor outputStyle, not style. Update each Sass task (around lines 87–88 and 97–98) to include:
options: {
implementation: require('sass'),
sourceMap: true,
- style: 'expanded'
+ style: 'expanded',
+ outputStyle: 'expanded'
}
…
options: {
implementation: require('sass'),
sourceMap: false,
- style: 'compressed'
+ style: 'compressed',
+ outputStyle: 'compressed'
}📝 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.
| style: 'expanded' | |
| } | |
| // … around lines 87–88 … | |
| options: { | |
| implementation: require('sass'), | |
| sourceMap: true, | |
| style: 'expanded', | |
| outputStyle: 'expanded' | |
| } | |
| // … around lines 97–98 … | |
| options: { | |
| implementation: require('sass'), | |
| sourceMap: false, | |
| style: 'compressed', | |
| outputStyle: 'compressed' | |
| } |
🤖 Prompt for AI Agents
In web/themes/custom/your_site_theme/Gruntfile.js around lines 87–88 (and also
update the similar block at lines 97–98), the sass task currently sets style:
'expanded' but grunt-sass v4 requires outputStyle; add outputStyle: 'expanded'
alongside the existing style key (and for the second block mirror the same
change, or set outputStyle to whatever the intended style is) so the task
options include both keys to ensure v4 respects the formatting option.
Summary by CodeRabbit