Repository navigation
[#581] Rendered the Drupal version and command wrapper text in generated projects from the installer answers. - #589
Merged
Conversation
…ted projects from the installer answers.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The change renders generated Drupal version and command wrapper text from the installer answers. No actionable merge-blocking risk was identified. Pre-merge checks ·
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 59.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 3 files. (30 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: rendering Drupal version and command-wrapper text from installer answers in generated projects. Linked Issues check Issue #581is directly linked and remains open. The change renderscore_version_requirement, thephpunit.xmlopening comment, and the Drupal-specificAGENTS.mdmatrix from selected majors. It re…Out of Scope Changes check The changes stay within issue #581. The installer helpers implement answer-dependent rendering. Template marker blocks, regenerated fixtures, documentation, and automated tests support that rendering …
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 59.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 3 files. (30 skipped: 30 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #589 +/- ##
==========================================
+ Coverage 91.70% 91.89% +0.19%
==========================================
Files 9 9
Lines 1579 1617 +38
Branches 4 4
==========================================
+ Hits 1448 1486 +38
Misses 131 131 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
added
the
Needs review
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
2 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.
Closes #581
Summary
install.phpnow writes the installer answers into the text that names the Drupal majors and the command wrappers, not only into which marker blocks stay.process()calls the newprocess_drupal_version_text()after the unselectedDRUPAL_<major>blocks go andprocess_command_wrapper_text()after the unselected wrapper blocks go, and each replaces the template's every-option text with whatcore_version_requirement(),phpunit_config_comment()andcommand_wrapper_text()return for the selected options. Text that fits on whole lines sits in blocks instead: theAGENTS.mdCI matrix inDRUPAL_<major>blocks, the commands for a project with no wrapper inDEV_NO_COMMAND_WRAPPERblocks, the template-only overview sentence in aMETAblock, and the.ahoy.yml,Makefileand[Makefile*]entries of.gitattributesand.editorconfiginDEV_AHOYandDEV_MAKEFILEblocks.With
INSTALL_DRUPAL_VERSION=10,11andINSTALL_COMMAND_WRAPPER=ahoy, the generated project still described every option. The*.info.ymlkeptcore_version_requirement: ^10 || ^11 || ^12, which declares Drupal 12 compatibility to Drupal.org although the CI matrix never tests it,phpunit.xmlopened with "Drupal 11 and 12 PHPUnit configuration", andAGENTS.mdsaid "PHP 8.3-8.5, Drupal 10-12", told agents to extend "themake/ahoytarget", listedmake build,make lintandmake testunder Development Workflow, headed the Ahoy commands "Using Ahoy (alternative)", opened with "This is a Force Crystal template for creating contributed modules or themes" and held a double blank line after the HARD RULE paragraph and before "Using Ahoy". With no command wrapper selected,AGENTS.mdkept a HARD RULE about wrappers the project doesn't have, 4 headings with no commands under them and the samemakeandahoyworkflow steps.After the merge, that Drupal 10 and 11 Ahoy project declares
core_version_requirement: ^10 || ^11, itsphpunit.xmlopens with<!-- Drupal 11 PHPUnit configuration. -->because Drupal 10 builds withphpunit.d10.xml, and itsAGENTS.mdlists "Drupal 10 on PHP 8.3-8.4" and "Drupal 11 on PHP 8.3-8.5", says "ahoy build" and "extend theahoytarget", and starts with "Force Crystal (force_crystal) is a Drupal module." ("theme" for a theme). A project with no wrapper getsscripts/eddy-tooling, thevendor/bin/eddy-*commands and a pointer toCONTRIBUTING.mdunder the same headings, andAGENTS.md,CONTRIBUTING.mdandREADME.mdhold no double blank line in any of the 8 selections thattestProcessLeavesNoDoubleBlankLines()renders.README.dist.md,phpunit.d10.xml,.github/workflows/test.ymland the.eddy/toolingcommands are unchanged, and the gaps that remain are listed under "Not in this PR".Before / After
Changes
The installer
install.php,process():process_drupal_version_text($selected_majors)runs right after theDRUPAL_<major>blocks of the unselected majors are removed, andprocess_command_wrapper_text($command_wrapper)runs right after the wrapper blocks are. Both callreplace_string_content()with the renderer's output for every supported option as the needle and its output for the selected options as the replacement, so a template edit that drifts from the every-option text leaves a needle that matches nothing and the text ships unrendered. The newInstallProcessTestdatasets render the real template for each selection, so that drift fails a test instead.process_drupal_version_text()renderscore_version_requirementin the extension's*.info.yml, such as^10 || ^11, and the comment that opensphpunit.xml. The comment names only the selected majors that have nophpunit.d<major>.xmlof their own, becauseeddy-assemblebuilds those majors with that file: Drupal 10 and 11 gives<!-- Drupal 11 PHPUnit configuration. -->, and Drupal 10 alone gives<!-- PHPUnit configuration. -->.process_command_wrapper_text()renders "makeorahoy" and, forbuild,lintandtest, "make buildorahoy build". Ahoy alone gives "ahoy" and "ahoy build". With no wrapper those phrases already sit inDEV_COMMAND_WRAPPERblocks, so the only replacement left is "The command wrappers and CI run it" becoming "CI runs it" in thescripts/eddy-toolingline of Project Structure.core_version_requirement(),phpunit_config_comment(),sort_drupal_majors()(numeric order with duplicates dropped, so a selection of12,10renders as^10 || ^12),command_wrapper_binaries()(makefiletomakeandahoytoahoy, in the order the template names them),command_wrapper_text()andjoin_list()(A, B and C). They take arrays and return strings, with no filesystem access.process_internal()replaces "is a Drupal module." with "is a Drupal theme." for a theme, next to the existingtype: modulereplacement.The template files
AGENTS.md, overview: the sentence about the scaffold, now followed by "install.phpturns the example extension below into yours.", sits in aMETAblock. The paragraph after it opens with "Your Extension (your_extension) is a Drupal module.", which the existing name replacements turn into " (<machine_name>) is a Drupal module."AGENTS.md, CI matrix: "Matrix testing" is 1 line per major inDRUPAL_10,DRUPAL_11andDRUPAL_12blocks, each with the PHP range of that major in.github/workflows/test.yml: Drupal 10 on PHP 8.3-8.4, Drupal 11 on PHP 8.3-8.5 and Drupal 12 on PHP 8.5.AGENTS.md, wrapper text: the HARD RULE paragraph says "themakeorahoytarget" where it said "make/ahoy", socommand_wrapper_text()can render it, and the "(default)" and "(alternative)" labels are gone, leaving "Using Make:" and "Using Ahoy:".AGENTS.md, no command wrapper:DEV_COMMAND_WRAPPERblocks wrap the HARD RULE paragraph, the lint and test lists and Development Workflow steps 1, 3 and 4, andDEV_NO_COMMAND_WRAPPERblocks give the same headings their direct commands:scripts/eddy-tooling,vendor/bin/eddy-assemble,eddy-start,eddy-stopandeddy-provision,build/vendor/bin/drush -l "$(vendor/bin/eddy-info site-url)"for Drush anduli, andvendor/bin/eddy-info. The lint and test commands stay inCONTRIBUTING.md, whereCONTRIBUTING.dist.mdalready lists them for this case, and the HARD RULE and workflow steps point there.AGENTS.md, blank lines: every marker block opens with the blank line that separates it from the text above, as inCONTRIBUTING.dist.md, so a removed block takes its blank line with it. A blank line on both sides of a block left a double blank line when the block went..gitattributesand.editorconfig: the.ahoy.ymlandMakefileexport-ignorelines and the[Makefile*]section sit inDEV_AHOYandDEV_MAKEFILEblocks, the way the tool entries in those files already did, so a project without a Makefile or an.ahoy.ymlno longer carries a line for it.Docs
.eddy/CLAUDE.md: a new "Text rendered from the answers" section lists whatcore_version_requirement(),phpunit_config_comment()andcommand_wrapper_text()render and what happens when a replacement finds nothing, "Rules for a block" gains the blank-line convention for Markdown blocks, and "Adding a Drupal major" grows from 6 steps to 7, addingcore_version_requirement, thephpunit.xmlcomment and theAGENTS.mdmatrix to what a new major needs.Tests
.eddy/tests/src/Unit/InstallHelpersTest.php: data providers forsort_drupal_majors(),core_version_requirement(),phpunit_config_comment(),command_wrapper_text()andjoin_list(), plusprocess_drupal_version_text()andprocess_command_wrapper_text()run against scratch files. The datasets cover an unordered selection, a major with its ownphpunit.d<major>.xml, text for other majors that has to stay, and a wrapper name the renderer doesn't know..eddy/tests/src/Unit/InstallProcessTest.php: renders the real template throughprocess()for the 7 non-empty Drupal major selections plus an unordered selection (testProcessRendersDrupalVersionText()) and for Ahoy, Makefile, both and neither wrapper (testProcessRendersWrapperText()). It also asserts the overview for a module and a theme, the.gitattributesand.editorconfigwrapper entries, thatAGENTS.mdmentionsmakeorahoyonly when that wrapper is selected, and thatAGENTS.md,CONTRIBUTING.mdandREADME.mdhold no double blank line in 8 selections..eddy/tests/fixtures/install/: regenerated snapshots, not hand-edited, for the datasets whoseAGENTS.md,*.info.yml,phpunit.xmlor.editorconfigoutput changed.Not in this PR
Follow-up candidates, left out on purpose:
README.dist.md: the PHP badges still show 8.3-8.5 for a Drupal 10-only or a Drupal 12-only project.phpunit.d10.xmlstill ships in projects without Drupal 10.AGENTS.md, "Code Quality Tools": the section is empty, with a double blank line, when every lint tool is removed.DRUPAL_VERSIONexamples inCONTRIBUTING.dist.mdandAGENTS.mdshow the variable's syntax with sample versions rather than claiming support, so they stay as they are.Makefilecomments that compare it with Ahoy.Testing
composer --working-dir=.eddy/tests test -- --group=p0: 1324 tests and 5378 assertions pass.composer --working-dir=.eddy/tests test -- --filter=InstallTest: 20 tests and 78 assertions pass against the regenerated snapshots.composer --working-dir=.eddy/tests lint: PHPCS, PHPStan and Rector report 0 issues across 89 files.ahoy lint(PHPCS, PHPStan, Rector, Twig CS Fixer, ESLint, Stylelint, CSpell) reports 0 issues, andahoy test-unitpasses its 4 tests.Summary by CodeRabbit