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.

Sign up for GitHub

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

Jump to bottom

Usage of ACM certificate instead of CloudFront #134

Merged
merged 6 commits into from
Aug 12, 2022
Merged

Usage of ACM certificate instead of CloudFront #134

merged 6 commits into from
Aug 12, 2022

Conversation

3h4x
Copy link
Contributor

@3h4x 3h4x commented Jun 8, 2022

No description provided.

@ofhouse
Copy link
Member

ofhouse commented Jun 9, 2022 •
edited
Loading

Hi, the changes proposed in your PR are too opinionated:

  • The CloudFront distribution would only work with custom domains
  • The CloudFront distribution would only work with custom certificates

Adding these settings as optional parameters would work, but we don't want to force our users into using a custom domain.

@3h4x
Copy link
Contributor Author

3h4x commented Jun 21, 2022

Hi, the changes proposed in your PR are too opinionated:

  • The CloudFront distribution would only work with custom domains
  • The CloudFront distribution would only work with custom certificates

Adding these settings as optional parameters would work, but we don't want to force our users into using a custom domain.

I totally agree. Unfortunately I dont have time to improve it.
I thought to push PR and even if it gets rejected it will leave a trace so other people could eventually pick it up.

@ofhouse
Copy link
Member

ofhouse commented Jun 23, 2022

@3h4x Thanks for clarifying!
Will try to pick this up for one of the next releases.

@ofhouse ofhouse added this to the Upcoming Iteration milestone Aug 12, 2022
@ofhouse ofhouse merged commit 0676cb1 into milliHQ:main Aug 12, 2022
@ofhouse
Copy link
Member

ofhouse commented Aug 12, 2022

This has been released in v12.1.3. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v12.1.3
Development

Successfully merging this pull request may close these issues.
2 participants
@3h4x @ofhouse