Modified letsencrypt support to add domain name checking in addition to#1509
Modified letsencrypt support to add domain name checking in addition to#1509erik-wramner merged 1 commit intodocker-mailserver:masterfrom neuralp:master
Conversation
|
There are changes not only for letsencrypt but also for the other alternatives. Are there real changes there, or is it only indentation? If there are real changes I need to review more carefully. There are some places where the if and fi are not aligned, probably you have mixed tabs and spaces. Can you fix that, please? I know that the script is full of indentation problems as is, but at least we can make sure the new parts are aligned. |
youtous
left a comment
youtous
left a comment
There was a problem hiding this comment.
Thank for the PR!
I've added some suggestion which could improve tests and error handling, some are just improvement seen the previous tests didn't used the suggested things.
I'm not sure how we should handle errors but it seems safer to me to use early return in case of error.
Since I was working on the SSL function, I figured that I would attempt to fix the indentation since it was all over the place. I'll revisit this for the function and make sure that all of the tabs get converted to spaces. Apparently my editor didn't read the .editorconfig. |
hostname checking. Added necessary tests and renamed original manual ssl test to a name that supports adding the other SSL tests.
|
Your suggestions have been taken into account and the PR has been modified to include these suggestions. Please review at your earliest convenience. |
|
Looks good, thanks! |
3 participants
hostname checking. Added necessary tests and renamed original manual
ssl test to a name that supports adding the other SSL tests.
Reference to issue #1274