-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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 configuration option for email sender address apart from SMTP username #5351
Conversation
Quality Gate passedIssues Measures |
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.
/ping @guqing
Please help us review the PR.
Hi @lingrottin, Thanks for your contribution, please fix the code style by Code Style and unit test cases first |
Hi, sorry for the late reply due to schooling issues. I have fixed the format issues and checked them with checkstyle. Additional unit tests is not required as this PR only modifies the inside of class |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5351 +/- ##
============================================
- Coverage 56.91% 56.54% -0.38%
+ Complexity 3319 3313 -6
============================================
Files 587 592 +5
Lines 18968 19204 +236
Branches 1401 1355 -46
============================================
+ Hits 10795 10858 +63
- Misses 7594 7784 +190
+ Partials 579 562 -17 ☔ View full report in Codecov by Sentry. |
Quality Gate passedIssues Measures |
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.
/lgtm
Thanks for your first contribution!
ping @halo-dev/sig-halo
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.
/lgtm
Thank you for first contribution!
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JohnNiang, ruibaby The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
/kind api-change
What this PR does / why we need it:
As described in #5350, when using Amazon SES, for example, as email provider, SMTP username for authentication may not exactly match sender email address. When this happens, websites using Halo will not be able to send emails due to invalid addresses that Halo put in SMTP requests. This PR adds a configuration field for those who got a non-email-address username to specify one separately.
Which issue(s) this PR fixes:
Fixes #5350
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Yes