Skip to content
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

#11 auto renew certificates #73

Merged

Conversation

mbican
Copy link
Contributor

@mbican mbican commented May 9, 2020

I just couldn't wait and I rebased my #11 renewal implementation, maybe somebody will find it useful. Anyway the @natemcmaster implementation mbican@f601519 seems smarter so ¯_(ツ)_/¯

@mbican mbican requested a review from natemcmaster as a code owner May 9, 2020 07:00
Copy link
Owner

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for picking this up. This is pretty much exactly how I was planning to do it but haven't had time to get around to it.

One minor nit-pick - I think there is an extra doc comment line that was accidentally merged in. Otherwise, the code looks great.

Thoughts on unit testing this? Obviously, we don't want to run tests for days or months, but I'd like to add some tests to this before releasing it to nuget. Doesn't have to hold up this PR, but maybe in a second step we can find a way to mock a system clock.

src/LetsEncrypt/LetsEncryptOptions.cs Outdated Show resolved Hide resolved
Co-authored-by: Nate McMaster <nate.mcmaster@gmail.com>
@mbican
Copy link
Contributor Author

mbican commented May 12, 2020

we need clock as a dependency for testing

@mbican mbican force-pushed the LE-11_Auto-renew_certificates branch from b886c86 to 368703c Compare May 12, 2020 14:33
Copy link
Owner

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@natemcmaster natemcmaster merged commit 983fc52 into natemcmaster:master May 13, 2020
@natemcmaster natemcmaster linked an issue May 13, 2020 that may be closed by this pull request
@natemcmaster natemcmaster mentioned this pull request May 26, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-renew certificates when close to expiring
2 participants