Open taophp opened 8 years ago
@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.
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.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.
Have you considered to use SwiftMailer in parallel of the build-in
mail
command? In fact, SwiftMailer is able to use themail
command, but give some good reasons to not to which apply to anymail
command use. They talk about their not-so-good experience with themail
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.