Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a typed autoinclude kind that tracks whether an autoinclude originates from a unit or a stack, changes filename selection to produce ChangesAutoinclude kind, filename, parsing, wiring, tests, and docs
Sequence Diagram(s)sequenceDiagram
rect rgba(30,144,255,0.5)
participant CLI as CLI
end
participant Parser as Parser
participant Generator as Generator
participant FS as Filesystem
participant Config as ConfigLoader
CLI->>Parser: parse config (detect autoinclude blocks)
Parser->>Parser: resolve autoinclude\n(tag as KindUnit or KindStack)
Parser->>Config: return ParseResult (AutoIncludes with Kind)
Config->>Generator: request GenerateAutoIncludeFile(resolved)
Generator->>Generator: AutoIncludeFileNameForKind(resolved.Kind)
Generator->>FS: write file named by kind (unit or .stack)
FS-->>Generator: write success/failure
Generator->>CLI: log output including filename and kind
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/data/changelog/v1.0.4/stack-autoinclude-filename-split.mdx`:
- Around line 13-34: The HCL example references undefined units (unit.vpc.path
and unit.shared.path) which will raise "Variable not found" during the two‑pass
autoinclude parse; fix by adding minimal stub unit declarations for the
referenced units (e.g., unit "vpc" and unit "shared") with matching source and
path values above the existing unit "app" and before stack "networking", or
alternatively add a clear inline note that those unit declarations are
intentionally omitted and the paths are illustrative only; ensure the stubs use
the same identifiers (unit.vpc and unit.shared) so the autoinclude dependency
lines (dependency "vpc" { config_path = unit.vpc.path } and dependency "shared"
{ config_path = unit.shared.path }) resolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7ec1c72-4683-4e77-ba2c-a8155edb1cd3
📒 Files selected for processing (9)
docs/src/data/changelog/v1.0.4/stack-autoinclude-filename-split.mdxdocs/src/data/experiments/stack-dependencies.mdxinternal/hclparse/autoinclude.gointernal/hclparse/generate.gointernal/hclparse/parse.gointernal/hclparse/parse_test.gopkg/config/autoinclude_test.gopkg/config/config.gopkg/config/stack.go
| ```hcl | ||
| # terragrunt.stack.hcl | ||
| unit "app" { | ||
| source = "../catalog/units/app" | ||
| path = "app" | ||
|
|
||
| autoinclude { | ||
| # Generated as: .terragrunt-stack/app/terragrunt.autoinclude.hcl | ||
| dependency "vpc" { config_path = unit.vpc.path } | ||
| } | ||
| } | ||
|
|
||
| stack "networking" { | ||
| source = "../catalog/stacks/networking" | ||
| path = "networking" | ||
|
|
||
| autoinclude { | ||
| # Generated as: .terragrunt-stack/networking/terragrunt.autoinclude.stack.hcl | ||
| dependency "shared" { config_path = unit.shared.path } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
HCL example references undefined units — copy-paste would error.
unit.vpc.path (line 21) and unit.shared.path (line 31) reference units that are not declared in the example. During the two-pass autoinclude parser, these would fail with a "Variable not found" diagnostic, which could confuse readers who copy the snippet.
Consider adding stub unit declarations or a note that the referenced units are omitted:
✏️ Proposed fix — add the missing unit stubs
```hcl
# terragrunt.stack.hcl
+unit "vpc" {
+ source = "../catalog/units/vpc"
+ path = "vpc"
+}
+
unit "app" {
source = "../catalog/units/app"
path = "app"
autoinclude {
# Generated as: .terragrunt-stack/app/terragrunt.autoinclude.hcl
dependency "vpc" { config_path = unit.vpc.path }
}
}
+unit "shared" {
+ source = "../catalog/units/shared"
+ path = "shared"
+}
+
stack "networking" {
source = "../catalog/stacks/networking"
path = "networking"
autoinclude {
# Generated as: .terragrunt-stack/networking/terragrunt.autoinclude.stack.hcl
dependency "shared" { config_path = unit.shared.path }
}
}</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/src/data/changelog/v1.0.4/stack-autoinclude-filename-split.mdx` around
lines 13 - 34, The HCL example references undefined units (unit.vpc.path and
unit.shared.path) which will raise "Variable not found" during the two‑pass
autoinclude parse; fix by adding minimal stub unit declarations for the
referenced units (e.g., unit "vpc" and unit "shared") with matching source and
path values above the existing unit "app" and before stack "networking", or
alternatively add a clear inline note that those unit declarations are
intentionally omitted and the paths are illustrative only; ensure the stubs use
the same identifiers (unit.vpc and unit.shared) so the autoinclude dependency
lines (dependency "vpc" { config_path = unit.vpc.path } and dependency "shared"
{ config_path = unit.shared.path }) resolve.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/hclparse/autoinclude.go`:
- Around line 56-57: Update the Resolve() doc comment to state that callers must
also set the Kind field (type AutoIncludeKind) after the call, just like
SourceBytes, because calling AutoIncludeFileNameForKind with the zero-value Kind
("") will panic; reference Resolve(), the Kind field, AutoIncludeKind,
SourceBytes, and AutoIncludeFileNameForKind in the comment so callers know to
populate Kind before using helpers that derive filenames.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c6f3346-5f93-4743-b090-472fbc1416bb
📒 Files selected for processing (6)
internal/hclparse/autoinclude.gointernal/hclparse/generate.gointernal/hclparse/parse.gointernal/hclparse/parse_test.gopkg/config/config.gopkg/config/stack.go
✅ Files skipped from review due to trivial changes (1)
- internal/hclparse/parse_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/hclparse/parse.go
- pkg/config/stack.go
- pkg/config/config.go
- internal/hclparse/generate.go
| // Kind is KindUnit or KindStack and drives the generated filename (terragrunt.autoinclude.hcl vs terragrunt.autoinclude.stack.hcl). | ||
| Kind AutoIncludeKind |
There was a problem hiding this comment.
Resolve() doc comment omits the Kind post-call requirement.
Resolve() already documents that callers must set SourceBytes after the call. The new Kind field carries the same obligation — if AutoIncludeFileNameForKind is called with the zero-value Kind (""), it panics immediately. The doc comment on Resolve() (lines 72–86) should add an equivalent notice for Kind, matching the existing SourceBytes guidance.
📝 Proposed doc update on `Resolve()`
// Callers that need to record the originating file's bytes on the returned
// AutoIncludeResolved (so generation can slice expressions from the correct
// source after include merging) should set SourceBytes on the result.
+//
+// Callers must also set Kind (KindUnit or KindStack) on the result before
+// passing it to AutoIncludeFileNameForKind; the zero value ("") is not valid
+// and will cause a panic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/hclparse/autoinclude.go` around lines 56 - 57, Update the Resolve()
doc comment to state that callers must also set the Kind field (type
AutoIncludeKind) after the call, just like SourceBytes, because calling
AutoIncludeFileNameForKind with the zero-value Kind ("") will panic; reference
Resolve(), the Kind field, AutoIncludeKind, SourceBytes, and
AutoIncludeFileNameForKind in the comment so callers know to populate Kind
before using helpers that derive filenames.
* feat: updated name for stack dependencies * chore: hcl parsing kind * chore: stack auto include file name
Description
Generated autoinclude filenames now depend on the component kind:
terragrunt.autoinclude.hclterragrunt.autoinclude.stack.hclRFC #5663
TODOs
Read the Gruntwork contribution guidelines.
Summary by CodeRabbit
Documentation
Tests