Conversation
WalkthroughUpdates multiple docs: replaces an AWS S3 example with a Suga SDK file API in docs/guides/add-suga.mdx; adds Mermaid class styling in docs/guides/aws/environment-management.mdx; converts the backend config guide to a Tabs-based interface in docs/guides/terraform-backend-config.mdx; inserts a Mermaid workflow diagram and minor wording changes in docs/introduction.mdx; and updates docs/quickstart.mdx with a template tip, language-tagged bash fences, template-specific wording, a Mermaid accomplishments diagram, and an added GitHub link. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sean-nitric
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/guides/aws/environment-management.mdx (2)
13-17: Fix typos in prerequisites.Polish user-facing text.
-This guide assumed you've have the following: +This guide assumes you have the following: ... -That you've build your Terraform configuration using `suga build`. +That you've built your Terraform configuration using `suga build`.
112-139: Add AWS credentials configuration in CI (OIDC) before Terraform.Without credentials, assume_role cannot execute. Add configure-aws-credentials with a role in the admin account that can assume the per-workspace roles.
- name: Determine Environment id: env run: echo "environment=${GITHUB_REF#refs/heads/}" >> $GITHUB_OUTPUT + + - name: Configure AWS Credentials (OIDC) + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ secrets.AWS_ADMIN_ROLE_ARN }} # Admin account role that can assume target roles + aws-region: us-east-1 - name: Terraform Init & Apply run: | cd terraform/stacks/<stack_name> terraform init terraform workspace select ${{ steps.env.outputs.environment }} || \ terraform workspace new ${{ steps.env.outputs.environment }} terraform apply -auto-approveAdd a note explaining AWS_ADMIN_ROLE_ARN must trust GitHub OIDC and have sts:AssumeRole to the target roles.
🧹 Nitpick comments (12)
docs/guides/terraform-backend-config.mdx (5)
56-58: Add a note on migrating existing local state.Users switching from local to a remote backend should run terraform init with -migrate-state to move existing state safely.
Apply this diff after the “Choose your backend provider…” sentence:
- Choose your backend provider and add the appropriate configuration: + Choose your backend provider and add the appropriate configuration: + <Tip> + If you’re migrating from local state, run <code>terraform init -migrate-state</code> to move your existing state to the remote backend. + </Tip>
58-75: S3 backend: recommend KMS for encryption.You’re enabling encrypt = true (SSE-S3). Suggest showing SSE-KMS to meet stricter compliance baselines.
backend "s3" { bucket = "my-terraform-state-bucket" key = "suga/terraform.tfstate" region = "us-east-1" - encrypt = true + encrypt = true + kms_key_id = "arn:aws:kms:us-east-1:ACCOUNT_ID:key/KEY_ID" # optional but recommended dynamodb_table = "terraform-locks" }
77-90: GCS backend: clarify prefix convention.Minor: note that prefix values typically end with a trailing slash to group state files.
- prefix = "suga/terraform/state" + prefix = "suga/terraform/state/"
92-103: Azure backend: include AAD auth option.Many CI setups use OIDC/AAD; include use_azuread_auth for completeness.
backend "azurerm" { resource_group_name = "terraform-state-rg" storage_account_name = "terraformstatestorage" container_name = "terraform-state" key = "suga.terraform.tfstate" + use_azuread_auth = true }
105-116: Terraform Cloud: surface hostname and workflow note.Add hostname for TFE/self-hosted and remind users to auth with terraform login.
backend "remote" { organization = "my-org" + hostname = "app.terraform.io" # or your TFE hostname workspaces { name = "suga-production" } }Add below the block:
+<Tip> + Authenticate with Terraform Cloud first: <code>terraform login</code>. +</Tip>docs/guides/aws/environment-management.mdx (1)
57-57: Convert bare URL to markdown link.Improves readability/accessibility.
-Terraform provides a great guide on getting started with this in their s3 backend configuration: https://developer.hashicorp.com/terraform/language/backend/s3#multi-account-aws-architecture +Terraform provides a great guide on getting started with this in their S3 backend configuration: [Multi-account AWS architecture](https://developer.hashicorp.com/terraform/language/backend/s3#multi-account-aws-architecture)docs/guides/add-suga.mdx (3)
186-253: Good diff; consider request-scoped timeouts for file writes.Guard IO with a short context timeout.
- router.HandleFunc("GET /hello/{name}", func(w http.ResponseWriter, r *http.Request) { + router.HandleFunc("GET /hello/{name}", func(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") - - // Simplified file write using Suga - if err := app.Files.Write("example.txt", []byte("Hello, "+name+"!")); err != nil { + // Simplified file write using Suga with a timeout + ctx, cancel := context.WithTimeout(r.Context(), 3*time.Second) + defer cancel() + if err := app.Files.WriteWithContext(ctx, "example.txt", []byte("Hello, "+name+"!")); err != nil { log.Printf("Failed to write file: %v", err) http.Error(w, "Error writing file", http.StatusInternalServerError) return }Add imports:
+ "context" + "time"(If WriteWithContext isn’t available, fall back to Write with a TODO comment.)
339-348: Add a title to the build output block for consistency.- ```bash + ```bash title="Build Output" icon="terminal"
368-381: Normalize stack path usage.Earlier you used ./.suga; use the same here.
- cd .suga/stacks/my-app-aws-* + cd ./.suga/stacks/my-app-aws-*docs/quickstart.mdx (3)
26-41: Consider adding a title to the interactive prompt block.- ```bash + ```bash title="Template Selection"
85-90: Add a title to the login confirmation block.- ```bash + ```bash title="Login Output"
98-103: Add a title to the editor launch block.- ```bash + ```bash title="Editor Launch"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/guides/add-suga.mdx(2 hunks)docs/guides/aws/environment-management.mdx(1 hunks)docs/guides/terraform-backend-config.mdx(5 hunks)docs/introduction.mdx(1 hunks)docs/quickstart.mdx(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-08T23:49:28.580Z
Learnt from: jyecusch
PR: nitrictech/suga#49
File: docs/quickstart.mdx:0-0
Timestamp: 2025-09-08T23:49:28.580Z
Learning: Suga currently only officially supports AWS (`suga/aws1`) and GCP (`suga/gcp1`) as deployment targets. Azure is not yet supported as a deployment target, despite `az login` being a valid Azure CLI authentication command.
Applied to files:
docs/introduction.mdx
📚 Learning: 2025-08-27T03:55:50.432Z
Learnt from: davemooreuws
PR: nitrictech/suga#29
File: docs/guides/terraform-backend-config.mdx:25-34
Timestamp: 2025-08-27T03:55:50.432Z
Learning: In the Suga project, the default directory for CDKTF synthesized output is `terraform/`, not `cdktf.out/`. The directory structure is terraform/stacks/<stack_name>/ containing the generated Terraform configurations.
Applied to files:
docs/quickstart.mdx
🔇 Additional comments (9)
docs/introduction.mdx (1)
9-21: Mermaid addition looks good and consistent with styling.docs/guides/aws/environment-management.mdx (1)
51-53: Diagram styling addition is clean and matches other pages.docs/guides/add-suga.mdx (1)
86-97: Verify Go base image tag exists; consider pinning to a stable minor.1.24-alpine may drift/not exist; pin to a known tag or ARG.
-FROM golang:1.24-alpine as builder +FROM golang:1.22-alpine as builder # or your verified versionWould you like me to check Docker Hub tags and open a follow-up PR?
docs/quickstart.mdx (6)
23-26: Nice tip addition; helps discovery of custom templates.
112-112: Template context callout is helpful.
159-174: Dev output block looks good; matches recent CLI.
212-223: Great: Bash fence and stack path clarity.
258-270: Apply block improves realism of the flow.
281-289: Mermaid “accomplishments” is clear and consistent with styling.
jyecusch
force-pushed
the
docs--review-of-v1-docs
branch
from
9003a4f to
fa6e4bc
Compare
jyecusch
force-pushed
the
docs--review-of-v1-docs
branch
from
fa6e4bc to
61d02e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/introduction.mdx (1)
52-53: Fix contradiction about database emulation.Top bullets say local DB emulation exists; later it’s “coming soon.” Align these. Based on prior learnings, DB emulation is not yet available.
-**Local cloud emulation** - Develop and test with local versions of cloud services (S3, databases, queues) +**Local cloud emulation** - Develop and test with local versions of cloud services (S3, queues; databases coming soon)(If DB emulation is now available, update Line 131 instead.)
Also applies to: 131-131
♻️ Duplicate comments (1)
docs/introduction.mdx (1)
7-7: Resolved: accurate cloud support statement.Thanks for aligning the intro with current support (AWS/GCP today; Azure coming soon). Matches prior feedback.
🧹 Nitpick comments (5)
docs/introduction.mdx (5)
8-8: Tighten “deploy anywhere” to avoid implying Azure today.Suggest wording to match actual support.
-Build applications with any framework, test locally with emulated cloud resources, and deploy anywhere with production-ready Terraform. +Build applications with any framework, test locally with emulated cloud resources, and deploy to AWS or GCP today (Azure coming soon) with production-ready Terraform.
9-21: Optional a11y: improve diagram contrast for dark mode.Mermaid looks good; consider adding fill/text color for better readability on dark backgrounds.
-classDef nitricGreen stroke:#25b355,stroke-width:2px +classDef nitricGreen stroke:#25b355,stroke-width:2px,fill:#0f2d1e,color:#ffffff
65-67: Avoid “any cloud provider” claim here.Match current support in this Step description.
-Deploy to any cloud provider with confidence, knowing your infrastructure follows best practices. +Deploy to AWS or GCP today (Azure coming soon) with confidence, knowing your infrastructure follows best practices.
78-80: Clarify Azure status and scope of “any cloud provider.”These bullets currently imply Azure parity and universal support.
-**Unified cloud API** - Replace AWS, GCP, or Azure SDKs with one consistent interface -**Production deployment** - Generate battle-tested Terraform for any cloud provider +**Unified cloud API** - Replace AWS or GCP SDKs with one consistent interface (Azure coming soon) +**Production deployment** - Generate battle-tested Terraform for AWS or GCP today (Azure coming soon)
154-158: Minor wording polish.“Kubernetes — Coming soon and BYO” reads a bit awkwardly.
-- **Kubernetes** - Coming soon and BYO +- **Kubernetes** - Coming soon (BYO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/guides/add-suga.mdx(2 hunks)docs/guides/aws/environment-management.mdx(1 hunks)docs/guides/terraform-backend-config.mdx(5 hunks)docs/introduction.mdx(1 hunks)docs/quickstart.mdx(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/guides/aws/environment-management.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/guides/terraform-backend-config.mdx
- docs/quickstart.mdx
- docs/guides/add-suga.mdx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-08T23:49:28.580Z
Learnt from: jyecusch
PR: nitrictech/suga#49
File: docs/quickstart.mdx:0-0
Timestamp: 2025-09-08T23:49:28.580Z
Learning: Suga currently only officially supports AWS (`suga/aws1`) and GCP (`suga/gcp1`) as deployment targets. Azure is not yet supported as a deployment target, despite `az login` being a valid Azure CLI authentication command.
Applied to files:
docs/introduction.mdx
|
🎉 This PR is included in version 0.0.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 participants
Minor updates to docs for readibility