Conversation
WalkthroughAdds a top-level Makefile target docs-dev (phony) that runs $(MAKE) -C docs dev. Introduces docs/Makefile with a phony dev target executing npx mint dev. Updates docs/docs.json to append guides/aws/environment-management to the Guides navigation group. Adds docs/guides/aws/environment-management.mdx detailing AWS multi-environment management using Terraform workspaces, including sections for overview, prerequisites, architecture (with mermaid diagram), provider/workspace configuration examples, workspace commands, CI/CD workflow, and references. No other files modified. Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
tjholm
force-pushed
the
env-management
branch
from
abe2e39 to
b474689
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
docs/Makefile (1)
1-4: Optional: pin CLI version and add a basic help target.Pinning avoids surprise breakages; a help target improves DX.
Example:
.PHONY: dev +DEFAULT_GOAL := dev + dev: - npx mint dev + npx -y mintlify@^4 devMakefile (1)
1-4: Optional: forward args and add a help description.Forwarding lets devs pass flags (e.g., --open).
.PHONY: docs-dev docs-dev: - $(MAKE) -C docs dev + $(MAKE) -C docs dev ARGS="$(ARGS)"And in docs/Makefile, reference ARGS in the command if adopted.
docs/guides/environment-management.mdx (5)
13-16: Fix typos in prerequisites.Grammar issues reduce polish.
-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`.
63-71: Add type and description for workspace_iam_roles.Improves validation and editor hints.
-variable "workspace_iam_roles" { +variable "workspace_iam_roles" { + description = "Mapping of Terraform workspace -> AWS IAM role ARN to assume" + type = map(string) default = {
73-79: Consider fail-fast instead of silent fallback to dev.Typos or unexpected workspace names could deploy to dev unintentionally. Prefer explicit allowlist + error.
provider "aws" { assume_role { - # Falls back to dev account if workspace doesn't exist (useful for PR previews) - role_arn = lookup(var.workspace_iam_roles, terraform.workspace, var.workspace_iam_roles["dev"]) + # Enforce known workspaces; error if unmapped + role_arn = var.workspace_iam_roles[terraform.workspace] } }If PR previews are needed, add an explicit mapping (e.g., “pr”) in the variable and derive it in CI.
73-79: Optional: include region and provider version for completeness.Prevents implicit-region surprises and pins provider.
Add to the example (either here or in versions.tf):
terraform { required_providers { aws = { source = "hashicorp/aws" version = "~> 5.0" } } } provider "aws" { region = var.aws_region assume_role { role_arn = var.workspace_iam_roles[terraform.workspace] } } variable "aws_region" { type = string description = "AWS region to deploy to" default = "us-east-1" }
109-136: Production safety: avoid blind auto-apply.Recommend plan + manual approval for production; auto-apply is fine for staging.
- - name: Terraform Init & Apply + - name: Terraform Init & Plan 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-approve + terraform plan -out tfplan + + - name: Manual Approval (production) + if: ${{ github.ref_name == 'production' }} + uses: trstringer/manual-approval@v1 + with: + secret: ${{ secrets.GITHUB_TOKEN }} + approvers: your-team + + - name: Terraform Apply + if: ${{ github.ref_name != 'production' || steps.manual-approval.outcome == 'success' }} + run: | + cd terraform/stacks/<stack_name> + terraform apply -auto-approve tfplan
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile(1 hunks)docs/Makefile(1 hunks)docs/docs.json(1 hunks)docs/guides/environment-management.mdx(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
docs/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🔇 Additional comments (5)
docs/Makefile (1)
3-4: Retainnpx mint dev– Mintlify’s official CLI docs specify the one-off install invocation asnpx mint dev(and globally asmint dev) (mintlify.com)Likely an incorrect or invalid review comment.
Makefile (1)
3-4: Looks good; simple and clear.Recursive make into docs is correct.
docs/docs.json (2)
44-46: Navigation entry looks correct.Path aligns with the new MDX file.
132-135: Verify PostHog key is intended to be public.
phc_...keys are typically client/public, but please confirm it’s safe to commit and scoped to docs only.docs/guides/environment-management.mdx (1)
60-79: Path and workspace naming caveat.Please confirm the stack path (
terraform/stacks/<stack_name>) matches Suga’s output. Also note: workspace names are[0-9A-Za-z_-]+; avoid slashes if deriving from branches.
tjholm
force-pushed
the
env-management
branch
2 times, most recently
from
a836783 to
46fd71d
Compare
raksiv
left a comment
raksiv
left a comment
There was a problem hiding this comment.
Looks great, as per the explaination of the configuration, the workflow could use a little more info, I've added a suggestion for review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/Makefile (1)
1-4: Pin Mintlify CLI for reproducible buildsAvoid floating npx installs; pin the CLI version and make it overridable.
.PHONY: dev +MINT_VERSION ?= 1.0.0 - -dev: - npx mint dev +dev: + npx -y mint@$(MINT_VERSION) devMakefile (1)
3-4: Allow passing extra args to docs dev (optional)Forward optional args (e.g., port) without hardcoding; harmless if unused.
docs-dev: - $(MAKE) -C docs dev + $(MAKE) -C docs dev $(DOCS_ARGS)Usage:
make docs-dev DOCS_ARGS="PORT=4000".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile(1 hunks)docs/Makefile(1 hunks)docs/docs.json(1 hunks)docs/guides/environment-management.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/guides/environment-management.mdx
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
docs/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🔇 Additional comments (2)
docs/Makefile (1)
1-4: LGTM on minimal docs dev targetPhony + simple dev command is clear.
Makefile (1)
3-4: LGTM: convenient wrapperThe recursive make into docs is straightforward and works as expected.
tjholm
force-pushed
the
env-management
branch
from
7f8c784 to
2983fad
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/Makefile (1)
1-4: Pin Mint CLI version and allow arg pass-through for reproducibility.Unpinned
npx mint devis non-deterministic. Pin a version and forward optional args.-.PHONY: dev - -dev: - npx mint dev +.PHONY: dev +MINT_VERSION ?= 1.0.x + +dev: + npx --yes mint@$(MINT_VERSION) dev $(ARGS)Makefile (1)
1-4: Optional: Pass ARGS through and add a docs-build target.Improves ergonomics without changing behavior.
-.PHONY: docs-dev +.PHONY: docs-dev docs-build docs-dev: - $(MAKE) -C docs dev + $(MAKE) -C docs dev ARGS="$(ARGS)" + +docs-build: + $(MAKE) -C docs build ARGS="$(ARGS)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile(1 hunks)docs/Makefile(1 hunks)docs/docs.json(1 hunks)docs/guides/aws/environment-management.mdx(1 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 (1)
- docs/docs.json
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
docs/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🔇 Additional comments (2)
docs/Makefile (1)
1-4: Optional: Add all/build targets (convenience + appease linters).PHONY: dev +.PHONY: all build dev: npx mint dev +all: dev + +build: + npx mint build $(ARGS)No
package.jsonfound to pinMINT_VERSION—verify your version fallback or explicitly defineMINT_VERSIONto avoid surprises.Makefile (1)
1-4: LGTM: simple trampoline to docs dev.Works as intended.
Co-authored-by: Rak <rak.siva@nitric.io>
tjholm
force-pushed
the
env-management
branch
from
2983fad to
78de518
Compare
|
🎉 This PR is included in version 0.0.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 participants
Not sure what else to include with this one without creating too much redundant documentation, or making this guide very long.
Fixes: NIT-184