-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Webhook] Added Sendgrid's webhook support (added in 6.4) #19267
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
Conversation
@@ -84,8 +84,14 @@ Mailer service Parser service name | |||
=============== ========================================== | |||
Mailgun ``mailer.webhook.request_parser.mailgun`` | |||
Postmark ``mailer.webhook.request_parser.postmark`` | |||
Sendgrid ``mailer.webhook.request_parser.sendgrid`` | |||
=============== ========================================== | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a versionadded for Mailgun and Postmark too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a third column, perhaps? To have a bit better overview when more are added in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we usually remove the versionadded directives when merging up to a new major, so we should stick with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Webhook itself is covered by a 6.3 versionadded in the top of the document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets do this in the PR of Alex #19268
Oh you were fast, just saw your PR. I proposed a different approach, but we can keep your PR as you opened first if you want 🙂 |
92d8c0d
to
b8d08cb
Compare
Thank you Timo. |
Follow-up of #19249