-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
apply patches in issues #129 and #124 #131
Conversation
README.md
Outdated
|
||
## Easy to use | ||
|
||
Providing an implementation of DMARC that SMTP utilities like SpamAssassin, amavis, and qpsmtpd can consume is expected to increase adoption. | ||
The effectiveness of DMARC will improve significantly as adoption increases. Proving an implementation of DMARC that SMTP utilities like SpamAssassin, amavis, and qpsmtpd can consume will aid adoption. |
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.
Should that read providing rather than proving?
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.
Yep. :)
bin/dmarc_send_reports
Outdated
@@ -375,7 +375,7 @@ sub email { | |||
}); | |||
$smtp->quit; | |||
return; | |||
} | |||
}; |
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.
Doesn't appear necessary
bin/dmarc_send_reports
Outdated
else { | ||
$report->store->error($rid, $err); | ||
}; | ||
}; |
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.
lines 414/415 terminating ; not necessary
A couple of comments made, generally looks good to me. |
Looks good. |
fixes #129
fixes #124
fixes #123
fixes #121