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

Use composer for dependency management #655

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

splittingred
Copy link
Contributor

Proposal: Add Composer for dependency management

Modern PHP apps utilize composer for dependency management to allow for external library usage, PSR-4 standards compatibility, and built-in autoloading.

This adds composer to webDip, which only adds one step to be run during initial install (composer install). Note: this would need to be run whenever the code is deployed to the actual server, along with installing composer on it first.

Thoughts? This could allow webDip to benefit from a bunch of external libraries (such as templating, API generation, serialization, DB access, caching, etc).

@diptobi1
Copy link
Contributor

diptobi1 commented Mar 3, 2020

I concur. The external phpMailer library already includes some traces of composer. And the library I would like to use for push notifications has some dependencies itself, too, that could be best managed with composer.

Webdip's software already uses an autoload-function itself for the variant extensions, though. And as far as I can judge, this would be broken by your current pull request (had the very same problem just a few days ago). Better add the original autoload function to the spl autoload queue with spl_autoload_register() in variants/variant.php.

@splittingred
Copy link
Contributor Author

@diptobi1 Good catch - thanks! I've updated the branch to consolidate the PHPMailer autoloader (since we're on PHP 7.0+ now), and adjusted the variant autoloader to use spl_autoload_register with a closure.

@TimothyJones
Copy link
Contributor

Awesome work!

@splittingred
Copy link
Contributor Author

@jmo1121109 @shdant113 Thoughts here?

@jmo1121109
Copy link
Contributor

Hey, at a first glance this looks really neat. I am not familiar with much in the way of php libraries, and anything being installed on the server has to run through @kestasjk, for security purposes, who is not currently active. So I do want to set the expectation that this may sit for a time.

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

Successfully merging this pull request may close these issues.

4 participants