hashover-next icon indicating copy to clipboard operation
hashover-next copied to clipboard

Use SwiftMailer

Open taophp opened this issue 9 years ago • 1 comments

Have you considered to use SwiftMailer in parallel of the build-in mail command? In fact, SwiftMailer is able to use the mail command, but give some good reasons to not to which apply to any mail command use. They talk about their not-so-good experience with the mail command and I have quite the same.

I tried this on my fork, but the code is based on a branch which included my two previous rejected pull requests, so I have some house keeping to do before making a new pull request... but you can already have a look here and tell me if you think this approach is interesting for you.

taophp avatar Jan 26 '16 21:01 taophp

@taophp

Sorry for the delay, I've been busy.

I'm not opposed to this at all. I am, however, opposed to how you've implemented it. It's early work, I understand, so don't take the following as harsh criticisms.

  • So firstly, I would like for the mail functionality to be modular, or rather, object-oriented. That is to say, all of the code related to SwiftMailer should be in the swiftMailerWrapper class. This means any settings that are directly related to SwiftMailer should be in that class as well, and any settings that could theoretically be used by different mail libraries (for example $SwiftMailerSmtpPort) should use more general names (for example $mailSmtpPort).

    See login.php and defaultlogin.php for a general idea of what I'm talking about.

  • You're using require_once to include the same file in two different places. This is inconsistent with the object oriented design of HashOver. If there is a file that is necessary to include it should be done once, likely in the swiftMailerWrapper class. I doubt this is necessary at all, as normally dependencies are installed in the operating system /usr/lib directory and made generally available to PHP in a friendlier way, and this should be the assumed setup if possible.

  • Many things need to be changed to conform to the coding standard. This isn't so much a task for you, as a warning up front that any code you contribute will be changed to conform.

  • You assigned only yourself on the swiftmailerwrapper.php Copyright. Accepting the file as-is would mean I would technically lack the necessary rights to distribute your contribution in HashOver under the GNU Affero General Public License.

jacobwb avatar Feb 02 '16 20:02 jacobwb