SEC-2528: Environment variable suppressing snapshot generation on installation.#24
Merged
nchiasson-dgi merged 3 commits intomainfrom Mar 10, 2026
Merged
SEC-2528: Environment variable suppressing snapshot generation on installation.#24nchiasson-dgi merged 3 commits intomainfrom
nchiasson-dgi merged 3 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThe install hook now checks the CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL environment variable and skips creating the content snapshot during module installation when that variable is truthy. README documentation was added to describe the new behavior and link to relevant source lines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.txt`:
- Line 24: Fix the README sentence for clarity and precision: replace the
typo-filled, ambiguous line about CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL
with a concise statement that lists accepted boolean values (e.g., "1", "true",
"yes", "on" to enable suppression and "0", "false", "no", "off" to disable),
removes the commit-pinned link, and references the drush command name
content-sync:snapshot as the alternative way to build the snapshot; ensure the
env var name CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL is spelled correctly and
the behavior is explicitly stated (when set to a truthy value the snapshot build
during module installation is skipped).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d78bc87-f172-46b1-a5bd-784261ea520a
📒 Files selected for processing (2)
README.txtcontent_sync.install
README.txt
Outdated
| Extract module at drupal/modules/contrib directory and enable it from browser | ||
| by going in this path /admin/modules. | ||
|
|
||
| The `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` environment variable might be set dto soemthing truthy in order to skip the build of the snapshot during module installation, with the expectation that the snapshot will be built by other means, such as the `drush content-sync:snapshot` command ( https://github.com/discoverygarden/content_sync/blob/7816728adc70b3c85642bd254b9096a44a1d0308/src/Drush/Commands/ContentSyncCommands.php#L519-L528 ). |
There was a problem hiding this comment.
Fix install note wording and make behavior precise.
Line 24 has typos and ambiguous wording ("truthy"). It also uses a commit-pinned link that will stale quickly. Please tighten this sentence and document accepted boolean values explicitly.
Suggested rewrite
-The `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` environment variable might be set dto soemthing truthy in order to skip the build of the snapshot during module installation, with the expectation that the snapshot will be built by other means, such as the `drush content-sync:snapshot` command ( https://github.com/discoverygarden/content_sync/blob/7816728adc70b3c85642bd254b9096a44a1d0308/src/Drush/Commands/ContentSyncCommands.php#L519-L528 ).
+Set `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` to a boolean true value (`1`, `true`, `on`, or `yes`) to skip snapshot build during module installation. Build it later with `drush content-sync:snapshot`.📝 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.
Suggested change
| The `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` environment variable might be set dto soemthing truthy in order to skip the build of the snapshot during module installation, with the expectation that the snapshot will be built by other means, such as the `drush content-sync:snapshot` command ( https://github.com/discoverygarden/content_sync/blob/7816728adc70b3c85642bd254b9096a44a1d0308/src/Drush/Commands/ContentSyncCommands.php#L519-L528 ). | |
| Set `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` to a boolean true value (`1`, `true`, `on`, or `yes`) to skip snapshot build during module installation. Build it later with `drush content-sync:snapshot`. |
🧰 Tools
🪛 LanguageTool
[style] ~24-~24: Consider a more concise word here.
Context: ...iable might be set dto soemthing truthy in order to skip the build of the snapshot during m...
(IN_ORDER_TO_PREMIUM)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.txt` at line 24, Fix the README sentence for clarity and precision:
replace the typo-filled, ambiguous line about
CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL with a concise statement that lists
accepted boolean values (e.g., "1", "true", "yes", "on" to enable suppression
and "0", "false", "no", "off" to disable), removes the commit-pinned link, and
references the drush command name content-sync:snapshot as the alternative way
to build the snapshot; ensure the env var name
CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL is spelled correctly and the behavior
is explicitly stated (when set to a truthy value the snapshot build during
module installation is skipped).
adam-vessey
changed the title
nhart
approved these changes
Mar 9, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.txt (1)
24-25:⚠️ Potential issue | 🟡 MinorClarify accepted values and avoid commit-pinned links.
The documentation has several clarity issues:
- "truthy" is ambiguous—users won't know which values enable suppression (e.g.,
1,true,yes,onvs.0,false,no,off)- The commit-pinned GitHub link will become stale when code moves or history changes
- The sentence is verbose and difficult to parse
📝 Suggested rewrite for clarity
-The `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` environment variable might be set to something truthy in order to skip the build of the snapshot during module installation, with the expectation that the snapshot will be built by other means such as the `drush content-sync:snapshot` command ( https://github.com/discoverygarden/content_sync/blob/7816728adc70b3c85642bd254b9096a44a1d0308/src/Drush/Commands/ContentSyncCommands.php#L519-L528 ). +Set `CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL` to a truthy value (e.g., `1`, `true`, `yes`, or `on`) to skip snapshot build during module installation. Build the snapshot later using `drush content-sync:snapshot`.As per static analysis, "in order to" can be simplified to "to" for conciseness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.txt` around lines 24 - 25, The sentence about CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL is unclear and links to a commit-pinned blob; update the README text to (1) replace "truthy" with an explicit list of accepted values (e.g., "set to 1, true, yes, or on to skip snapshot build; set to 0, false, no, or off to build it"), (2) simplify wording by using "to" instead of "in order to", and (3) remove the commit-specific GitHub blob link—either reference the drush command name ("drush content-sync:snapshot") or link to the repository/file at a branch or tag URL that won't become stale; mention the drush command (content-sync:snapshot) so readers know the alternative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.txt`:
- Around line 24-25: The sentence about
CONTENT_SYNC__SUPPRESS_SNAPSHOT_ON_INSTALL is unclear and links to a
commit-pinned blob; update the README text to (1) replace "truthy" with an
explicit list of accepted values (e.g., "set to 1, true, yes, or on to skip
snapshot build; set to 0, false, no, or off to build it"), (2) simplify wording
by using "to" instead of "in order to", and (3) remove the commit-specific
GitHub blob link—either reference the drush command name ("drush
content-sync:snapshot") or link to the repository/file at a branch or tag URL
that won't become stale; mention the drush command (content-sync:snapshot) so
readers know the alternative.
|
Tag generated by PR: v3.1.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Snapshots might be rather intensive to build, and
hook_install()can't kick off batches, so: Let's introduce an environment variable to allow the snapshot generation during module install to be suppressed, with the expectation of separate process being used to populate the snapshot, such as adrush content-sync:snapshot(which executes batch-wise).Summary by CodeRabbit
New Features
Documentation