-
Notifications
You must be signed in to change notification settings - Fork 0
fix: use ALIAS records for top level custom domains #83
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
fix: use ALIAS records for top level custom domains #83
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes consist of updates to two documentation files related to the SettleMint platform. The Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Reviewer's Guide by SourceryThis pull request updates the documentation for custom domain configuration in custom deployments and makes a minor formatting change in the components documentation. The main focus is on changing the DNS record type from CNAME to ALIAS for top-level domains and providing more detailed instructions for DNS configuration. User journey diagram for updated DNS configuration processjourney
title Updated DNS Configuration Process for Custom Domains
section Access DNS Settings
User: Log in to domain registrar or DNS provider's control panel
section Update DNS Records
User: Remove existing A and AAAA records for the domain
User: Remove existing A and AAAA records for the www domain
User: Create ALIAS records pointing to the provided hostname
section Configure Subdomains
User: Create additional CNAME records for subdomains if needed
section Set TTL
User: Set a lower TTL for quicker propagation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @janb87 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The change from CNAME to ALIAS records is a good improvement. However, there's an unexplained change in the example hostnames from 'your-app.settlemint-platform.com' to 'gke-europe.settlemint.com'. Could you provide clarification on this change or ensure consistency in the examples?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
docs/using-platform/24_custom-deployment.md (2)
39-39: Consider using a code block for DNS record examplesThe examples are clear and correct. To improve readability, consider using a code block for the DNS record examples. This will make it easier for users to distinguish the examples from the surrounding text.
Here's a suggested format:
- - Example: `ALIAS example.com gke-europe.settlemint.com` and `ALIAS www.example.com gke-europe.settlemint.com` + - Example: + ``` + ALIAS example.com gke-europe.settlemint.com + ALIAS www.example.com gke-europe.settlemint.com + ```
43-43: Consider using a code block for the CNAME record exampleThe example is clear and correct. To maintain consistency with the previous suggestion and improve readability, consider using a code block for the CNAME record example.
Here's a suggested format:
- - Example: `CNAME app.example.com gke-europe.settlemint.com` + - Example: + ``` + CNAME app.example.com gke-europe.settlemint.com + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- docs/about-settlemint/4_components.mdx (0 hunks)
- docs/using-platform/24_custom-deployment.md (1 hunks)
💤 Files with no reviewable changes (1)
- docs/about-settlemint/4_components.mdx
🧰 Additional context used
🪛 Markdownlint
docs/using-platform/24_custom-deployment.md
37-37: null
Bare URL used(MD034, no-bare-urls)
🔇 Additional comments (2)
docs/using-platform/24_custom-deployment.md (2)
36-38: LGTM: Clear instructions for DNS configurationThe updated instructions for DNS configuration are clear and accurate. Removing existing A and AAAA records before creating ALIAS records is a crucial step to avoid conflicts and ensure proper routing.
🧰 Tools
🪛 Markdownlint
37-37: null
Bare URL used(MD034, no-bare-urls)
Line range hint
1-1: Summary: Improved DNS configuration instructionsThe changes in this file significantly improve the DNS configuration instructions for custom domains in the SettleMint platform. The use of ALIAS records for top-level domains is clearly explained, which aligns with the PR objective. The instructions are accurate and provide users with the necessary steps to properly configure their custom domains.
A couple of minor formatting suggestions were made to enhance readability, but overall, the changes are well-implemented and valuable for users.
🧰 Tools
🪛 Markdownlint
37-37: null
Bare URL used(MD034, no-bare-urls)
Summary by Sourcery
Update documentation to recommend using ALIAS records instead of CNAME records for top-level custom domains and provide clearer instructions for DNS configuration.
Documentation:
Summary by CodeRabbit