feat: Refactor localization to YAML catalogs with runtime language overrides#6506
feat: Refactor localization to YAML catalogs with runtime language overrides#6506Copilot wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Atlantis comment localization from hardcoded strings to embedded YAML catalogs, and adds support for operator-provided runtime language overrides via a custom YAML catalog file (without rebuilding the binary).
Changes:
- Added
server/i18npackage with embedded YAML catalogs (en,es) plus merge/override behavior for custom catalogs. - Wired language selection and custom catalog path through CLI flags/config into
events.MarkdownRenderer. - Added Spanish template overrides under
server/events/templates/i18n/es/and expanded unit tests + docs for the new config options.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| server/user_config.go | Adds language and language-config-file to server user config. |
| server/server.go | Wires i18n.TranslatorConfig into markdown renderer construction. |
| server/i18n/locales/en.yaml | Adds built-in English YAML catalog. |
| server/i18n/locales/es.yaml | Adds built-in Spanish YAML catalog. |
| server/i18n/i18n.go | Implements catalog parsing, merging, language normalization/validation, and translator. |
| server/i18n/i18n_test.go | Adds unit tests for normalization, validation, built-in loading, and custom override behavior. |
| server/events/templates/i18n/es/wrapped_err.tmpl | Spanish override for wrapped error rendering. |
| server/events/templates/i18n/es/unwrapped_err.tmpl | Spanish override for unwrapped error rendering. |
| server/events/templates/i18n/es/single_project_plan_unsuccessful.tmpl | Spanish override for single-project unsuccessful plan template. |
| server/events/templates/i18n/es/single_project_plan_success.tmpl | Spanish override for single-project successful plan template (with instructions). |
| server/events/templates/i18n/es/single_project_apply.tmpl | Spanish override for single-project apply template. |
| server/events/templates/i18n/es/multi_project_plan.tmpl | Spanish override for multi-project plan template. |
| server/events/templates/i18n/es/multi_project_plan_footer.tmpl | Spanish override for plan footer summary/instructions. |
| server/events/templates/i18n/es/multi_project_header.tmpl | Spanish override for multi-project header template. |
| server/events/templates/i18n/es/multi_project_apply.tmpl | Spanish override for multi-project apply template. |
| server/events/templates/i18n/es/multi_project_apply_footer.tmpl | Spanish override for apply footer summary. |
| server/events/templates/i18n/es/log.tmpl | Spanish override for verbose log template. |
| server/events/templates/i18n/es/failure.tmpl | Spanish override for failure template. |
| server/events/markdown_renderer.go | Loads localized template sets; uses translator for command/vcs labels; introduces config-based localization. |
| server/events/markdown_renderer_test.go | Adds tests for Spanish localization and custom catalog overrides. |
| runatlantis.io/docs/server-configuration.md | Documents --language and --language-config-file with schema and behavior. |
| cmd/server.go | Adds flags/env wiring, defaults, and validation for language + custom catalog. |
| cmd/server_test.go | Adds validation coverage for language selection and custom catalog errors. |
Comments suppressed due to low confidence (1)
server/events/markdown_renderer.go:281
policyCheckResultsWrapped/policyCheckResultsUnwrappedtemplates gate some content behindif eq .Command "Policy Check". With localization/custom command titles,common.Commandis no longer guaranteed to be "Policy Check", so those sections will be skipped for policy_check results. Consider passing a stable (non-localized) command title into thepolicyCheckResultsDataused for these templates so the condition remains correct regardless of localization.
} else if result.PolicyCheckResults != nil && matchesCommandTitle(common.Command, policyCheckCommandTitle, m.translator) {
policyCheckResults := policyCheckResultsData{
PreConftestOutput: result.PolicyCheckResults.PreConftestOutput,
PostConftestOutput: result.PolicyCheckResults.PostConftestOutput,
PolicyCheckResults: *result.PolicyCheckResults,
Code ReviewReviewed +996/-51 across 30 files. Two distinct changes bundled: localization framework + unrelated yaml.v3 merge-key panic fix. Strengths
IssuesHIGH — Bundled unrelated changes. PR mixes localization feature (~950 lines) with yaml.v3 merge-key panic fix (~70 lines + fuzz). Should be 2 PRs. Reviewers and MEDIUM — Variadic config is non-idiomatic. func NewMarkdownRenderer(..., localizationConfigs ...i18n.TranslatorConfig) *MarkdownRendererVariadic for an optional single value is an anti-pattern; only first element used, extras silently ignored. Prefer MEDIUM — func MustNewTranslator(config TranslatorConfig) *Translator {
translator, err := NewTranslator(config)
if err == nil { return translator }
translator, _ = NewTranslator(TranslatorConfig{LanguageCode: DefaultLanguage})
if translator != nil { return translator }
return &Translator{...}
}Name suggests panic on failure; it never panics. Rename MEDIUM — Custom catalog YAML not protected from same panic. LOW — retErr = fmt.Errorf("yaml: unexpected error while parsing: %v", r)For an upstream library bug, debugging benefits from stack. Consider attaching LOW — Embed glob fragility. //go:embed templates/*.tmpl templates/i18n/*/*.tmplNew locale directory with typo (e.g. LOW — Spanish strings need native review.
Security / PerformanceNo security concerns. Catalog path is operator-supplied via CLI flag, not user input — no traversal risk. Translator built once at startup; map lookups O(1). Negligible runtime cost. Recommendations
|
|
Regarding the approach, supporting the same templates for different languages seems like it's going to be a hassle to manage and error prone. A common approach is to only manage the localization strings in a single file per language and the templating would be similar for all. What do you think of this logic? |
7c88eff to
cc0b267
Compare
…errides Add configurable Atlantis pull request comment localization with built-in English and Spanish catalogs plus optional custom YAML catalog overrides. Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> Signed-off-by: Rui Chen <rui@chenrui.dev>
cc0b267 to
2a268e6
Compare
interface{}->any)go test ./server/core/config/...)make check-fmt)decodeYAMLdoc clarity for yaml.v3 merge-key panic handlinggolangci-lint run,go test ./server/core/config/... ./server/events/...)