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

Add dkim support to smtp adapter #74

Merged
merged 1 commit into from
May 3, 2016
Merged

Conversation

nicksanders
Copy link
Contributor

dkim config can be added to config or email.provider_options or both, dkim config in email.provider_options overrides the one in the config.

@stevedomin
Copy link
Member

@nicksanders do you have a use-case for overriding the DKIM options on the per-email basis? If not I'd be quite keen on keeping it simple for now.

@nicksanders
Copy link
Contributor Author

@stevedomin That's fine, I wanted the ability to use different dkim configs depending on the sender domain but I can update the config on deliver. I have also added dkim support to sendmail.

@stevedomin
Copy link
Member

Thank you so much @nicksanders!!

As a heads up, I might end up extracting the common functionalities from SMTP and Sendmail in a separate module. Haven't figured out where it should live yet.

@stevedomin stevedomin merged commit c29bd4f into swoosh:master May 3, 2016
@nicksanders
Copy link
Contributor Author

Yeah, I had thought that as well, something like RawEmail
On 3 May 2016 12:26 p.m., "Steve Domin" notifications@github.com wrote:

Thank you so much @nicksanders https://github.com/nicksanders!!

As a heads up, I might end up extracting the common functionalities from
SMTP and Sendmail in a separate module. Haven't figured out where it should
live yet.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#74 (comment)

@barisbalic
Copy link
Collaborator

👍 on the extraction, documenting my opinion even though we've talked about it :D

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.

3 participants