Composer Manager isn't really needed anymore.

CommentFileSizeAuthor
#5 composer-manager-soft-dependency.patch1.24 KBrobloach

Comments

RobLoach created an issue. See original summary.

perignon’s picture

It's not required for 8.x, or at least it isn't supposed to be.

http://cgit.drupalcode.org/sendgrid_integration/tree/sendgrid_integratio...

Did you find it requiring composer manager when you installed the module?

robloach’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev

Meant for Drupal 7 ;-) ... Composer Manager should likely be a soft-dependency. Optional, if you will....

Been using Drupal Composer Drupal Project to set up Drupal 7 sites, so the sendgrid library gets installed without the need of Composer Manager. I could shoot you a patch next week.

perignon’s picture

Composer manager for 7.x is a hard requirement. You are using a custom Drupal installation based on Composer - which is cool. But that is an edge case. The module is developed to work on a default installation of Drupal 7.

robloach’s picture

Title: Remove Composer Manager Usage » Make Composer Manager a Soft-Dependency
Status: Active » Needs review
StatusFileSize
new1.24 KB

This makes Composer Manager a soft-dependency. It recommends its use, but you're not forced to use it. Do you have a better way to inform that? Maybe in the description?

perignon’s picture

Status: Needs review » Needs work

I cannot commit this patch because it will break the installation of the module. Without the wrapper API and something to alert the user that it does not exist the module will not work - then no one is happy.

A better approach will be to use a hook_requirements so the module cannot be installed. I would be in favor of implementing the suggestion of this issue if the module is blocked from being used. A hook_requirements in the install phase the prohibits the installation of the module if the API wrapper class does not exist (class_exists()) will be the best solution. This way they can choose to use composer manager to load the dependency or load it via some other method. I can write this later today or tomorrow.

robloach’s picture

A better approach will be to use a hook_requirements so the module cannot be installed.

Oh, I like that.

  • Perignon committed 33e6cc5 on 7.x-1.x
    Issue #2859348 by RobLoach, Perignon: Use hook_requirements to check for...

  • Perignon committed 955f908 on 7.x-1.x
    Issue #2859348 by RobLoach, Perignon: Update docuemntation for...
perignon’s picture

Status: Needs work » Needs review

I pushed updates to 7.x-1.x-dev that have the hook_requirements change. I have tested this with a site that is using Composer Manager to load the dependencies. Can you test in your environment using the Drupal Composer Drupal Project?

perignon’s picture

Crap... this change broke my Travis builds. I will address those later. I relied on the .info to load composer-manager and pull in the API wrapper. Will have to force loading composer-manager before scripts are run in Travis.

robloach’s picture

http://cgit.drupalcode.org/sendgrid_integration/commit/?id=0a71139

Ah, yes, that's important. I'll try the latest out. Nice work on this one!

perignon’s picture

Fixed the Travis builds. Back to green :-)

perignon’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.