Skip to content
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

[module:install] Allow module installation with Drupal 8.8.x and enable Composer-based installation #4167

Merged
merged 3 commits into from
Oct 10, 2019

Conversation

mondrake
Copy link
Contributor

@mondrake mondrake commented Oct 2, 2019

This PR fixes #4166 and allows to use the --composer option in module:install.

Adds some demonstration of that in TravisCI: the Imagemagick module in dev-3.x adheres strictly to the core_version_requirement syntax, so here the TravisCI build shows how to download that module (together with others) with the --composer option and get it installed.

Note that using the --composer option you can also use Composer syntax with constraints to specify the module to be installed. The four examples below are equivalent:

$ drupal module:install --composer token

$ drupal module:install --composer token:^1

$ drupal module:install --composer drupal/token

$ drupal module:install --composer "drupal/token:^1"

@enzolutions
Copy link
Contributor

@mondrake why the --composer is not possible in version 8.7.7 and lowers?

I am a little hesitant and include in the code validation for specific versions.

@mondrake
Copy link
Contributor Author

@enzolutions actually —composer is working ALWAYS with this PR. You can see that in the TravisCI runs for PHP 5.5 and 5.6, wher Drupal 8.6 is installed 😃

What becomes version dependent with this PR is the call to moduleRequirements that uses old APIs. Actually I believe that entire method is redundant, since Drupal module installer service just performs the same checks.

I would remove it, but prefer to do so in a separate PR

@enzolutions
Copy link
Contributor

So, you plan to merge this PR and in the next one remove the redundancy?

@mondrake
Copy link
Contributor Author

yes. later on, there’s also work to do for module update to match what we are doing in module install

@mondrake
Copy link
Contributor Author

@enzolutions - if you want, take a look at mondrake/drupal-console:dev-moi-modulereq-remove for a change that includes both this PR and the removal of the moduleRequirement method. I can submit it as a PR if you want to make a bigger jump. TravisCI passes on that one, too.

https://github.com/hechoendrupal/drupal-console/compare/master...mondrake:dev-moi-modulereq-remove?expand=1

@enzolutions
Copy link
Contributor

OK, but the other link, doesn't include the removal of this code

 if (version_compare(\Drupal::VERSION, '8.7.7') < 0) {
            $this->moduleRequirement($module);
        }

@mondrake
Copy link
Contributor Author

😀 because in the other link it never gets there vs current HEAD

@mondrake
Copy link
Contributor Author

sorry if I confused more than clarified... with this PR the call is conditional based on Drupal bersion. With the other link the call and methid are removed.

if this PR gets merged, then the following PR will remove the if... and the call inside.

@enzolutions enzolutions merged commit 9af887a into hechoendrupal:master Oct 10, 2019
@enzolutions
Copy link
Contributor

done @mondrake

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants