Closed (fixed)
Project:
OG Mailinglist
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 Sep 2010 at 22:38 UTC
Updated:
19 Oct 2010 at 22:00 UTC
Other users of the PHPMailer library expect to find it in a path called phpmailer. The attached patch modifies our behaviour to match.
| Comment | File | Size | Author |
|---|---|---|---|
| 0002-Make-PHPMailer-library-path-consistent.patch | 1.15 KB | nigelcunningham |
Comments
Comment #1
nigelcunningham commentedComment #2
darrellduane commentedI think this is related to http://drupal.org/node/925928
What do you think about this approach, NIgel ? I agree we need consistency! Should we do it just by referencing a standard library location or should we do it by requiring a module ? I'd suggest the latter, it feels more concrete and obvious and easy to generate error messages. I feel like it also simplifies the installation procedure by requiring an additional module rather than just referencing a somewhat arbitrary library.
Comment #3
kyle_mathews commentedDarrell -- depending on the PHPMailer module solves nothing. It just adds another (divergent) step to the installation process. The PHPMailer module doesn't ship with PHPMailer. And its purpose is quite a bit different from our own.
Look at the PHPMailer README.txt - http://drupalcode.org/viewvc/drupal/contributions/modules/phpmailer/READ...
It instructs you to download and install PHPMailer to the libraries directory just like ogm.
If there's some other reason you think it'd be a good idea to depend on the PHPMailer module, then please contribute that back in your original issue. But it doesn't simplify installation.
But back to the point of this issue. I noticed that the PHPMailer module also instructs you to download the PHPMailer library to a "phpmailer" folder. So apparently that is the convention for libraries, to make the folder lowercase. So I've applied your patch and committed it at http://github.com/KyleAMathews/og_mailinglist/commit/55812fe0afa8e162cdd...
Comment #4
kyle_mathews commentedComment #5
nigelcunningham commentedSorry, but I agree with Kyle, Darrell.
If the PHPMailer module came with PHPMailer itself, that would be a different story, but Kyle is right - adding the dependency would solve nothing and actually make og_mailinglist more difficult to install (by requiring a module it doesn't actually need).
Regards,
Nigel
Comment #6
darrellduane commentedHi Nigel & Kyle,
Thanks for your dialogue on this. I still think that we should go with the dependency because there are going to be many sites that have other modules that rely on the PHPMailer library.
If installation issues change or new configurations become available for the PHPMailer Library, the PHPMailer Module will (in theory) be there for us. OGM should focus on mailing lists, and let the greater community of users of modules that rely on the PHPMailer library keep the PHPMailer Module up to date. Drupal's sucess comes in part to its modularity and distribution of effort. Parcelling out the management of the PHPMailer library to another module helps keep things simpler in the long run.
Also, its just good manners. Should a site require the PHPMailer library for something else, should the administrator have to download it and install it a second time? By sticking to an agreed upon location for this library, all modules can share in its existence, and an administrator can do one upgrade as a new versions come out.
Kyle, thanks for changing the title of my original issue: http://drupal.org/node/925928, and keeping it active, I just don't think it should be optional for OGM. Had we gone ahead and relied on it in the first place the issue around capitalization wouldn't have occurred.
Comment #7
lp commentedI couldn't be much more out of my league here, but I figure I'd mention http://drupal.org/project/libraries - should it be of any interest, if only for roadmapping. P'raps you all know if it already..
Comment #8
kyle_mathews commentedDarrell, it seems you're confusing the PHPMailer module with the PHPMailer library.
Look at the PHPMailer module page --http://drupal.org/project/phpmailer It's sole purpose for being is for providing support for people who'd like to use an external SMTP server for sending emails. This isn't something most people using ogm need.
As you point out, not requiring people to reinstall PHPMailer for ogm is good manners. So we don't. As LP pointed out, the Libraries module provides a way for multiple modules to share a common installation of a library with the libraries folder. The PHPMailer module supports this as of their 3.x branch as has ogm since quite awhile ago. The original point of this issue was Nigel pointing out that ogm's install directory within the libraries folder for phpmailer was different. But since that's been changed, if you already have the PHPMailer library installed for another module, you don't have to install anything extra when installing ogm.
And to reiterate why I think the PHPMailer module should be optional. It's sole purpose for being is to support advanced email sending configurations, which the vast majority of users of ogm won't need. So there's no reason to require an extra module for configuring the PHPMailer library when most people won't need to configure it, it'll just work out of the box with their server configuration. But I agree that we should support it for people who do want to do advanced SMTP configurations.