Support atmos docs generate feature natively #934
Conversation
TODO
|
mergify
bot
added
conflict
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
internal/exec/docs_generate.go (5)
104-104: 🛠️ Refactor suggestionConsider making 'README.yaml' filename configurable
Instead of hardcoding 'README.yaml', allow the filename to come from the configuration for greater flexibility.
117-117: 🛠️ Refactor suggestionUpdate error message for clarity
Change the error message to reflect that we're generating Terraform docs within atmos, not running an external command.
-return fmt.Errorf("failed to run terraform-docs: %w", err) +return fmt.Errorf("failed to generate terraform docs: %w", err)
156-169: 🛠️ Refactor suggestionReuse existing HTTP fetch methods
Use the existing methods (like
go-getter) for HTTP fetching to handle authentication (e.g.,GITHUB_TOKEN) and avoid duplicating code.
239-239: 🛠️ Refactor suggestionUse consistent HTTP fetch methods for templates
Consider using the same HTTP fetching mechanisms used elsewhere to handle authentication and maintain consistency.
276-295: 🛠️ Refactor suggestionLeverage existing deep merge functionality
Reuse atmos's native deep merging capabilities (possibly using
mergo) instead of implementing a customdeepMergefunction.
🧹 Nitpick comments (6)
cmd/docs_generate.go (1)
9-9: Fix typo in commentCorrect the typo: "genrates" should be "generates".
-// docsGenerateCmd genrates README.md +// docsGenerateCmd generates README.mdinternal/exec/template_utils.go (2)
226-234: Add function documentation.Consider adding a detailed function documentation comment that explains:
- The purpose of the function
- Parameter descriptions
- Return value descriptions
- Usage examples
+// ProcessTmplWithDatasourcesGomplate parses and executes Go templates with datasources using Gomplate. +// It processes the template specified by tmplName and tmplValue using the provided merged data. +// +// Parameters: +// - atmosConfig: The Atmos configuration +// - settingsSection: Settings for template processing +// - tmplName: Name of the template +// - tmplValue: The template content +// - mergedData: Data to be used in template processing +// - ignoreMissingTemplateValues: If true, missing values are replaced with empty strings +// +// Returns: +// - string: The processed template +// - error: Any error that occurred during processing func ProcessTmplWithDatasourcesGomplate(
246-259: Enhance error handling for temporary file operations.The temporary file operations could benefit from more robust cleanup and detailed error messages.
tmpfile, err := os.CreateTemp("", "gomplate-data-*.json") if err != nil { - return "", fmt.Errorf("failed to create temp data file for gomplate: %w", err) + return "", fmt.Errorf("failed to create temporary data file for gomplate (pattern: gomplate-data-*.json): %w", err) } tmpName := tmpfile.Name() -defer os.Remove(tmpName) +defer func() { + if err := os.Remove(tmpName); err != nil { + // Log the error but don't fail the operation + u.LogWarning(atmosConfig, fmt.Sprintf("failed to remove temporary file %s: %v", tmpName, err)) + } +}()pkg/schema/schema.go (2)
195-204: Add field documentation for TerraformDocsSettings.Consider adding documentation comments for each field to explain their purpose and valid values.
type TerraformDocsSettings struct { + // Enabled determines if Terraform documentation generation is active Enabled bool `yaml:"enabled,omitempty" json:"enabled,omitempty" mapstructure:"enabled,omitempty"` + // Format specifies the output format (e.g., "markdown", "asciidoc") Format string `yaml:"format,omitempty" json:"format,omitempty" mapstructure:"format,omitempty"` + // ShowProviders controls whether provider documentation is included ShowProviders bool `yaml:"show_providers,omitempty" json:"show_providers,omitempty" mapstructure:"show_providers,omitempty"` + // ShowInputs controls whether input variable documentation is included ShowInputs bool `yaml:"show_inputs,omitempty" json:"show_inputs,omitempty" mapstructure:"show_inputs,omitempty"` + // ShowOutputs controls whether output variable documentation is included ShowOutputs bool `yaml:"show_outputs,omitempty" json:"show_outputs,omitempty" mapstructure:"show_outputs,omitempty"` + // SortBy defines the sorting criteria for documentation elements (e.g., "name", "required") SortBy string `yaml:"sort_by,omitempty" json:"sort_by,omitempty" mapstructure:"sort_by,omitempty"` + // HideEmpty determines if empty sections should be omitted from the output HideEmpty bool `yaml:"hide_empty,omitempty" json:"hide_empty,omitempty" mapstructure:"hide_empty,omitempty"` + // IndentLevel specifies the number of spaces to use for indentation IndentLevel int `yaml:"indent_level,omitempty" json:"indent_level,omitempty" mapstructure:"indent_level,omitempty"` }
206-211: Enhance DocsGenerate struct with documentation and validation.Consider adding field documentation and validation tags to ensure proper usage.
type DocsGenerate struct { + // Input is a list of paths to input files (e.g., README.yaml) + // At least one input file must be specified Input []string `yaml:"input,omitempty" json:"input,omitempty" mapstructure:"input"` + // Template is a list of template files to be used for generation + // At least one template file must be specified Template []string `yaml:"template,omitempty" json:"template,omitempty" mapstructure:"template"` + // Output specifies the path where the generated documentation will be written Output string `yaml:"output,omitempty" json:"output,omitempty" mapstructure:"output"` + // Terraform contains settings specific to Terraform documentation generation Terraform TerraformDocsSettings `yaml:"terraform,omitempty" json:"terraform,omitempty" mapstructure:"terraform"` }atmos.yaml (1)
398-404: Consider making paths more configurable.The hardcoded paths could be made more flexible by supporting:
- Environment variable substitution
- Relative path resolution
- Multiple file patterns
input: - - "./README.yaml" + - "${ATMOS_README_INPUT:-./README.yaml}" + - "**/*.yaml" # Support recursive search template: - - "./README.md.gotmpl" + - "${ATMOS_README_TEMPLATE:-./README.md.gotmpl}" + - "https://raw.githubusercontent.com/org/repo/main/templates/*.gotmpl" output: "README.md"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
atmos.yaml(1 hunks)cmd/docs_generate.go(1 hunks)go.mod(8 hunks)internal/exec/docs_generate.go(1 hunks)internal/exec/file_utils.go(2 hunks)internal/exec/template_utils.go(8 hunks)pkg/schema/schema.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: Build (macos-latest, macos)
internal/exec/file_utils.go
[failure] 7-7:
url redeclared in this block
[failure] 7-7:
"net/url" imported and not used
[failure] 12-12:
filepath redeclared in this block
[failure] 9-9:
other declaration of filepath
[failure] 12-12:
"path/filepath" imported and not used
[failure] 13-13:
runtime redeclared in this block
[failure] 10-10:
other declaration of runtime
[failure] 13-13:
"runtime" imported and not used
[failure] 14-14:
strings redeclared in this block
🪛 GitHub Check: Build (windows-latest, windows)
internal/exec/file_utils.go
[failure] 7-7:
url redeclared in this block
[failure] 7-7:
"net/url" imported and not used
[failure] 12-12:
filepath redeclared in this block
[failure] 9-9:
other declaration of filepath
[failure] 12-12:
"path/filepath" imported and not used
[failure] 13-13:
runtime redeclared in this block
[failure] 10-10:
other declaration of runtime
[failure] 13-13:
"runtime" imported and not used
[failure] 14-14:
strings redeclared in this block
🪛 GitHub Check: Build (ubuntu-latest, linux)
internal/exec/file_utils.go
[failure] 7-7:
url redeclared in this block
[failure] 7-7:
"net/url" imported and not used
[failure] 12-12:
filepath redeclared in this block
[failure] 9-9:
other declaration of filepath
[failure] 12-12:
"path/filepath" imported and not used
[failure] 13-13:
runtime redeclared in this block
[failure] 10-10:
other declaration of runtime
[failure] 13-13:
"runtime" imported and not used
[failure] 14-14:
strings redeclared in this block
🪛 GitHub Actions: Tests
internal/exec/file_utils.go
[error] 7-7: url redeclared in this block
go.mod
[error] 53-69: Module dependencies are not properly ordered. Run 'go mod tidy' to fix the ordering and commit the changes.
🪛 yamllint (1.35.1)
atmos.yaml
[error] 413-413: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
cmd/docs_generate.go (1)
23-32: Enhance readability of command examplesWrap command examples in code fences for better formatting in the help output.
Examples: - Generate a README.md in the current path: + ``` atmos docs generate + ``` - Generate a README.md for the VPC component: + ``` atmos docs generate components/terraform/vpc + ``` - Generate all README.md (recursively searches for README.yaml to rebuild docs): + ``` atmos docs generate --all + ```atmos.yaml (2)
397-398: Address TODO comment regarding README.yaml location logic.The TODO comment indicates missing functionality for finding README.yaml files. Consider implementing this before merging.
Would you like me to help implement the logic for finding README.yaml files based on the repository owner/organization structure?
400-402: Address TODO comment regarding remote templates.The TODO comment suggests that remote template support is planned. Consider implementing this capability now.
Would you like me to help implement support for remote template URLs?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exec/file_utils.go (2)
101-124: Strong implementation with clear documentation! 💪The function effectively handles path conversion for both Windows and Linux systems, with excellent documentation and examples.
Consider optimizing the Windows path handling.
The Windows path handling could be simplified using an unconditional
TrimPrefix:- if strings.HasPrefix(pathSlashed, "/") { - pathSlashed = strings.TrimPrefix(pathSlashed, "/") - } + pathSlashed = strings.TrimPrefix(pathSlashed, "/")🧰 Tools
🪛 golangci-lint (1.62.2)
112-112: S1017: should replace this
ifstatement with an unconditionalstrings.TrimPrefix(gosimple)
126-142: Consider making the drive letter check more explicit! 🎯The drive letter check on line 136 could be more explicit about its purpose:
- if strings.HasPrefix(u.Path, "/") && len(u.Path) > 2 && u.Path[2] == ':' { + // Check if path starts with a drive letter (e.g., "/C:/...") + if strings.HasPrefix(u.Path, "/") && len(u.Path) > 2 && + ((u.Path[1] >= 'A' && u.Path[1] <= 'Z') || (u.Path[1] >= 'a' && u.Path[1] <= 'z')) && + u.Path[2] == ':' {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(9 hunks)internal/exec/file_utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- go.mod
🧰 Additional context used
🪛 golangci-lint (1.62.2)
internal/exec/file_utils.go
112-112: S1017: should replace this if statement with an unconditional strings.TrimPrefix
(gosimple)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
|
💥 This pull request now has conflicts. Could you fix it @Listener430? 🙏 |
|
This PR was closed due to inactivity and merge conflicts. 😭 |
|
💥 This pull request now has conflicts. Could you fix it @Listener430? 🙏 |
|
This PR was closed due to inactivity and merge conflicts. 😭 |
|
💥 This pull request now has conflicts. Could you fix it @Listener430? 🙏 |
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
🛑 Comments failed to post (8)
internal/exec/docs_generate.go (8)
68-68:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Same issue as above. Remove the extra parameter:
- u.LogDebug(rootConfig, "No 'docs.generate.template' is defined, generating minimal readme.") + u.LogDebug("No 'docs.generate.template' is defined, generating minimal readme.")📝 Committable suggestion
‼️ 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.u.LogDebug("No 'docs.generate.template' is defined, generating minimal readme.")🧰 Tools
🪛 GitHub Check: golangci
[failure] 68-68: [golangci] internal/exec/docs_generate.go#L68
too many arguments in call to u.LogDebug
[failure] 68-68: [golangci] internal/exec/docs_generate.go#L68
too many arguments in call to u.LogDebug
[failure] 68-68: [golangci] internal/exec/docs_generate.go#L68
too many arguments in call to u.LogDebug
[failure] 68-68: [golangci] internal/exec/docs_generate.go#L68
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 68-68:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 68-68:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 68-68:
too many arguments in call to u.LogDebug
221-221:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Apply the same fix:
- u.LogDebug(atmosConfig, fmt.Sprintf("Error reading template file: %v. Using fallback.", err)) + u.LogDebug(fmt.Sprintf("Error reading template file: %v. Using fallback.", err))📝 Committable suggestion
‼️ 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.u.LogDebug(fmt.Sprintf("Error reading template file: %v. Using fallback.", err))🧰 Tools
🪛 GitHub Check: golangci
[failure] 221-221: [golangci] internal/exec/docs_generate.go#L221
too many arguments in call to u.LogDebug
[failure] 221-221: [golangci] internal/exec/docs_generate.go#L221
too many arguments in call to u.LogDebug
[failure] 221-221: [golangci] internal/exec/docs_generate.go#L221
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 221-221:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 221-221:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 221-221:
too many arguments in call to u.LogDebug
123-123:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Same pattern requires fixing:
- u.LogDebug(rootConfig, fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", subDir, localErr)) + u.LogDebug(fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", subDir, localErr))📝 Committable suggestion
‼️ 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.u.LogDebug(fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", subDir, localErr))🧰 Tools
🪛 GitHub Check: golangci
[failure] 123-123: [golangci] internal/exec/docs_generate.go#L123
too many arguments in call to u.LogDebug
[failure] 123-123: [golangci] internal/exec/docs_generate.go#L123
too many arguments in call to u.LogDebug
[failure] 123-123: [golangci] internal/exec/docs_generate.go#L123
too many arguments in call to u.LogDebug
[failure] 123-123: [golangci] internal/exec/docs_generate.go#L123
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 123-123:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 123-123:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 123-123:
too many arguments in call to u.LogDebug
169-169:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Adjust the call to meet the function signature:
- u.LogDebug(rootConfig, fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", targetDir, localErr)) + u.LogDebug(fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", targetDir, localErr))📝 Committable suggestion
‼️ 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.u.LogDebug(fmt.Sprintf("Error loading local atmos.yaml in %s: %v. Using root config instead.", targetDir, localErr))🧰 Tools
🪛 GitHub Check: golangci
[failure] 169-169: [golangci] internal/exec/docs_generate.go#L169
too many arguments in call to u.LogDebug
[failure] 169-169: [golangci] internal/exec/docs_generate.go#L169
too many arguments in call to u.LogDebug
[failure] 169-169: [golangci] internal/exec/docs_generate.go#L169
too many arguments in call to u.LogDebug
[failure] 169-169: [golangci] internal/exec/docs_generate.go#L169
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 169-169:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 169-169:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 169-169:
too many arguments in call to u.LogDebug
214-214:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Same fix approach:
- u.LogDebug(atmosConfig, fmt.Sprintf("Error fetching template '%s': %v. Using fallback instead.", docsGenerate.Template, err)) + u.LogDebug(fmt.Sprintf("Error fetching template '%s': %v. Using fallback instead.", docsGenerate.Template, err))📝 Committable suggestion
‼️ 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.u.LogDebug(fmt.Sprintf("Error fetching template '%s': %v. Using fallback instead.", docsGenerate.Template, err))🧰 Tools
🪛 GitHub Check: golangci
[failure] 214-214: [golangci] internal/exec/docs_generate.go#L214
too many arguments in call to u.LogDebug
[failure] 214-214: [golangci] internal/exec/docs_generate.go#L214
too many arguments in call to u.LogDebug
[failure] 214-214: [golangci] internal/exec/docs_generate.go#L214
too many arguments in call to u.LogDebug
[failure] 214-214: [golangci] internal/exec/docs_generate.go#L214
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 214-214:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 214-214:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 214-214:
too many arguments in call to u.LogDebug
253-253:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogInfo.Adjust to avoid extra parameter:
- u.LogInfo(atmosConfig, fmt.Sprintf("Generated docs at %s", outputPath)) + u.LogInfo(fmt.Sprintf("Generated docs at %s", outputPath))📝 Committable suggestion
‼️ 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.u.LogInfo(fmt.Sprintf("Generated docs at %s", outputPath))🧰 Tools
🪛 GitHub Check: golangci
[failure] 253-253: [golangci] internal/exec/docs_generate.go#L253
too many arguments in call to u.LogInfo
[failure] 253-253: [golangci] internal/exec/docs_generate.go#L253
too many arguments in call to u.LogInfo
[failure] 253-253: [golangci] internal/exec/docs_generate.go#L253
too many arguments in call to u.LogInfo🪛 GitHub Check: Build (macos-latest, macos)
[failure] 253-253:
too many arguments in call to u.LogInfo🪛 GitHub Check: Build (windows-latest, windows)
[failure] 253-253:
too many arguments in call to u.LogInfo🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 253-253:
too many arguments in call to u.LogInfo
65-65:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogDebug.Encountered pipeline error. Update the call to match the function signature:
- u.LogDebug(rootConfig, "No 'docs.generate.input' sources defined in atmos.yaml.") + u.LogDebug("No 'docs.generate.input' sources defined in atmos.yaml.")📝 Committable suggestion
‼️ 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.u.LogDebug("No 'docs.generate.input' sources defined in atmos.yaml.")🧰 Tools
🪛 GitHub Check: golangci
[failure] 65-65: [golangci] internal/exec/docs_generate.go#L65
too many arguments in call to u.LogDebug
[failure] 65-65: [golangci] internal/exec/docs_generate.go#L65
too many arguments in call to u.LogDebug
[failure] 65-65: [golangci] internal/exec/docs_generate.go#L65
too many arguments in call to u.LogDebug
[failure] 65-65: [golangci] internal/exec/docs_generate.go#L65
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (macos-latest, macos)
[failure] 65-65:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (windows-latest, windows)
[failure] 65-65:
too many arguments in call to u.LogDebug🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 65-65:
too many arguments in call to u.LogDebug🪛 GitHub Actions: Tests
[error] 65-65: too many arguments in call to u.LogDebug
188-188:
⚠️ Potential issueFix build failure: too many arguments in call to
u.LogTrace.Please update accordingly:
- u.LogTrace(atmosConfig, fmt.Sprintf("Skipping input '%s' due to error: %v", src, err)) + u.LogTrace(fmt.Sprintf("Skipping input '%s' due to error: %v", src, err))📝 Committable suggestion
‼️ 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.u.LogTrace(fmt.Sprintf("Skipping input '%s' due to error: %v", src, err))🧰 Tools
🪛 GitHub Check: golangci
[failure] 188-188: [golangci] internal/exec/docs_generate.go#L188
too many arguments in call to u.LogTrace
[failure] 188-188: [golangci] internal/exec/docs_generate.go#L188
too many arguments in call to u.LogTrace
[failure] 188-188: [golangci] internal/exec/docs_generate.go#L188
too many arguments in call to u.LogTrace
[failure] 188-188: [golangci] internal/exec/docs_generate.go#L188
too many arguments in call to u.LogTrace🪛 GitHub Check: Build (macos-latest, macos)
[failure] 188-188:
too many arguments in call to u.LogTrace🪛 GitHub Check: Build (windows-latest, windows)
[failure] 188-188:
too many arguments in call to u.LogTrace🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 188-188:
too many arguments in call to u.LogTrace
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/exec/docs_generate.go (1)
112-112:⚠️ Potential issueCheck the error return value of
os.Chdir.Ignoring a potential error may leave the process in an unexpected directory state.
Possible fix:
- defer os.Chdir(oldDir) // ensure we go back no matter what + defer func() { + if errChdir := os.Chdir(oldDir); errChdir != nil { + log.Debug("Failed to restore working directory", "error", errChdir) + } + }()🧰 Tools
🪛 golangci-lint (1.62.2)
112-112: Error return value of
os.Chdiris not checked(errcheck)
🪛 GitHub Check: golangci
[failure] 112-112: [golangci] internal/exec/docs_generate.go#L112
Error return value ofos.Chdiris not checked (errcheck)
🧹 Nitpick comments (9)
internal/exec/docs_generate.go (9)
23-23: Use the required alias for the charmbracelet/log import.According to the static analysis configuration, you must alias
"github.com/charmbracelet/log"aslog.Here's a sample diff:
- "github.com/charmbracelet/log" + log "github.com/charmbracelet/log"🧰 Tools
🪛 golangci-lint (1.62.2)
23-23: import "github.com/charmbracelet/log" imported without alias but must be with alias "log" according to config
(importas)
🪛 GitHub Check: golangci
[failure] 23-23: [golangci] internal/exec/docs_generate.go#L23
import "github.com/charmbracelet/log" imported without alias but must be with alias "log" according to config (importas)
92-93: Consider passing large structs by pointer.Several function parameters accept very large struct types (e.g.,
schema.AtmosConfiguration,schema.ConfigAndStacksInfo). Passing them by pointer may enhance performance by avoiding copies of large data structures. This is a good-to-have optimization rather than a strict requirement.Also applies to: 146-149, 180-182, 259-259, 313-313
🧰 Tools
🪛 GitHub Check: golangci
[failure] 92-92: [golangci] internal/exec/docs_generate.go#L92
hugeParam: rootConfig is heavy (5880 bytes); consider passing it by pointer (gocritic)
[failure] 93-93: [golangci] internal/exec/docs_generate.go#L93
hugeParam: rootInfo is heavy (1088 bytes); consider passing it by pointer (gocritic)
179-179: Split or shorten thegenerateSingleReadmefunction.This function exceeds the recommended maximum line count, which may hinder readability and maintainability.
🧰 Tools
🪛 GitHub Check: golangci
[failure] 179-179: [golangci] internal/exec/docs_generate.go#L179
function-length: maximum number of lines per function exceeded; max 60 but got 72 (revive)
222-222: Reduce repeated string literals.The literal
"error"appears multiple times. Consider using a named constant or more descriptive phrases.🧰 Tools
🪛 GitHub Check: golangci
[failure] 222-222: [golangci] internal/exec/docs_generate.go#L222
add-constant: string literal "error" appears, at least, 4 times, create a named constant for it (revive)
258-258: End comment with a period.The doc comment should end with a period to adhere to style guidelines.
-// fetchAndParseYAML fetches a YAML file from a local path or URL, parses it, and returns the data +// fetchAndParseYAML fetches a YAML file from a local path or URL, parses it, and returns the data.🧰 Tools
🪛 GitHub Check: golangci
[failure] 258-258: [golangci] internal/exec/docs_generate.go#L258
Comment should end in a period (godot)
268-268: Run gofmt to address formatting issues.Static analysis indicates that line 268 is not properly formatted. Use
go fmtor your IDE's auto-formatter to fix.🧰 Tools
🪛 GitHub Check: golangci
[failure] 268-268: [golangci] internal/exec/docs_generate.go#L268
File is not properly formatted (gofmt)
338-338: Include a period at the end of the comment.For consistency, please terminate the code comment with a period.
🧰 Tools
🪛 GitHub Check: golangci
[failure] 338-338: [golangci] internal/exec/docs_generate.go#L338
Comment should end in a period (godot)
363-366: Use static errors with wrapping instead of dynamic.Consider using predeclared errors or wrapped static errors for clarity and maintainability.
- return "", fmt.Errorf("multiple files found in %s (found %d)", dir, len(files)) + return "", fmt.Errorf("%w: multiple files found in %s (found %d)", ErrMultipleFiles, dir, len(files))🧰 Tools
🪛 GitHub Check: golangci
[failure] 363-363: [golangci] internal/exec/docs_generate.go#L363
do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf("no files found in %s", dir)" (err113)
[failure] 366-366: [golangci] internal/exec/docs_generate.go#L366
do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf("multiple files found in %s (found %d)", dir, len(files))" (err113)
371-371: Add a period at the end of this comment.Helps maintain consistent doc comments.
🧰 Tools
🪛 GitHub Check: golangci
[failure] 371-371: [golangci] internal/exec/docs_generate.go#L371
Comment should end in a period (godot)
🛑 Comments failed to post (1)
internal/exec/docs_generate.go (1)
250-250: 🛠️ Refactor suggestion
Tighten file permissions when writing README.
Use
0600or stricter file permissions for sensitive output to comply with secure coding standards.- if err = os.WriteFile(outputPath, []byte(rendered), 0o644); err != nil { + if err = os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil {📝 Committable suggestion
‼️ 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.if err = os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil {🧰 Tools
🪛 GitHub Check: golangci
[failure] 250-250: [golangci] internal/exec/docs_generate.go#L250
G306: Expect WriteFile permissions to be 0600 or less (gosec)
what
This PR implements a native atmos command to generate readmes like this
atmos docs generateto generate readmes for the current folder,atmos docs generate --allto generate readmes for the current and all subfolders.Terraform docs generation is also supported. To enable it, set respective parameters in the docs section of atmos.yaml.
E.g. this one generates both the readmes and components/variables descriptions based on the output of terraform-doc.
atmos docs generate examples\quick-start-advanced\components\terraform\vpcTesting
Refer to tests\test-cases\docs-generate.yaml
--- PASS: TestCLICommands (0.99s) --- PASS: TestCLICommands/atmos_docs_generate_in_current_directory (0.18s) --- PASS: TestCLICommands/atmos_docs_generate_with_subdirectories (0.44s) --- PASS: TestCLICommands/atmos_docs_generate_with_path_as_agrument (0.18s) --- PASS: TestCLICommands/atmos_docs_generate_for_terraform_docs (0.19s)why
Have an option to run a command natively from within atmos (not using builld-harness repo)
references
Summary by CodeRabbit
Based on the comprehensive changes, here are the updated release notes:
New Features
atmos docs generateREADME.mdfiles from YAML configurationsgeneratecommandImprovements
Dependencies
The release introduces a powerful new documentation generation feature for Atmos, allowing users to automatically create and customize README files for their components and stacks.