-
-
Notifications
You must be signed in to change notification settings - Fork 46
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 mailfrom
and smtpserver
parameters
#37
Conversation
(MODULES-3860) Add `mailfrom` and `smtpserver` parameters representing the mrepo `mailfrom` and `smtp-server` configuration settings.
# validate email addresses based on regex found in `is_email_address` | ||
# in newer stdlib versions. If metadata.json updated to require | ||
# *4.12.0* or newer, the `validate_re` calls can be replaced by | ||
# `validate_email_address`. |
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.
@jearls Thanks for your PR! I'd be OK with updating the dependency on stdlib to 4.12.0
Modernize `mailto` parameter to use `undef` instead of the string literal `'UNSET'`. Change `mailfrom` and `smtpserver` to conform. Use `validate_email_address` to validate `mailto` and `mailfrom` parameters.
@@ -121,6 +128,13 @@ | |||
validate_bool($rhn) | |||
validate_hash($descriptions) | |||
|
|||
if $mailto != undef { |
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.
This can be simplified as if $mailto
if $mailto != undef { | ||
validate_email_address($mailto) | ||
} | ||
if $mailfrom != undef { |
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.
Same here (if $mailfrom
)
mailto = <%= mailto %> | ||
<% if @mailto -%> | ||
mailto = <%= @mailto %> | ||
<% if @mailfrom -%> |
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.
Some indentation wonkiness here?
<% if @mailfrom -%>
to
<% if @mailfrom -%>
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 indented <% if @mailfrom -%>
is to indicate that that is inside the surrounding <% if @mailto -%>
block. The mailfrom
and smtp-server
parameters only make sense if mailto
is set.
I could indent the <%
itself, but I'd have to transform it to <%-
, and that also looks strange when the parameters inside that if
block are not themselves indented.
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.
Ah I see. Yeah seems to be a matter of taste, but both make sense 👍
mailto = <%= mailto %> | ||
<% if @mailto -%> | ||
mailto = <%= @mailto %> | ||
<% if @mailfrom -%> |
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.
Ah I see. Yeah seems to be a matter of taste, but both make sense 👍
@jearls Thanks! |
@jearls Thanks for your contribution! 👍 |
(MODULES-3860) Add
mailfrom
andsmtpserver
parameters representingthe mrepo
mailfrom
andsmtp-server
configuration settings.