Closed (fixed)
Project:
Custom Markup Block
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
1 Mar 2020 at 21:52 UTC
Updated:
18 Apr 2020 at 11:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
manuel.adanComment #4
sahana _n commentedComment #5
sahana _n commentedThe patch is created Please review.
Comment #7
sahana _n commentedUpdated the patch, please review.
Comment #8
manuel.adanThanks for your update. Adding an interdiff file is appreciated for patch reviewing.
@@ -1,6 +1,6 @@
-core: 8.x
Removing this statement makes the module incompatible with Drupal < 8.7.7, it should be present by now.
It seems that adding default Drupal core itself to the module composer requirements is not mandatory. The module developer guide about creating a composer.json file does not (currently) mention it, but many of the most installed modules has no such dependency declaration, unless the module is not compatible with some specific core versions.
For instance, the composer.json file of the webform module has no drupal/core listed in the requirement section, but it is present in the package information ( composer show -a drupal/webform ).
There are some indentation errors.
Some garbage seems to be accidentally added here.
Remove the version indication is right, it is commented out by the Drupal package system, but should be addressed in a different issue.
According to this CR, the recommended default theme is Stark and should be used when possible.
The install configuration of the block_test modules depends on the classy theme in Drupal < 9.0, so its value should be initialized during the setup depending on the Drupal core version.
Comment #9
manuel.adanComment #10
sahana _n commented@Manuel Adán Thanks for the review.
I created a new patch with your suggestions, please review the patch.
Comment #11
manuel.adanDefault theme is now dynamically set based on the core main version.
Comment #13
manuel.adanWrong patch syntax.
Comment #15
manuel.adan