Introduce functional options for configuration construction.#198
Conversation
- Added `WithValue` for templated access to custom key-value pairs. - Introduced `.Value` variable for configuration templates. - Replaced older configuration methods with `New` for a unified interface. - Updated documentation, examples, and changelog accordingly. - Upgraded `golang.org/x/tools` to v0.47.0. Signed-off-by: Alexander Adam <alphaone23@gmail.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 53 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors config construction to a functional options API via ChangesFunctional Options and WithValue Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
config_test.go (2)
420-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the new overlay value path in this test.
This
WithValue("does", "nothing")input is never consumed by either YAML source, so a regression inconfig.go's value propagation during overlay would still pass here. Make one of the overlaid sources template from the injected value, or switch one source to inline templated YAML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config_test.go` around lines 420 - 423, The test setup in the templig.New call is not actually exercising value propagation through overlay because WithValue("does", "nothing") is unused by both YAML inputs. Update config_test.go so one of the overlaid sources in the templig.New path references the injected value via templating, or replace one WithFile source with inline templated YAML, and keep the relevant symbols templig.New, WithValue, and WithFile in place to verify the overlay value path.
859-863: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the sentinel error instead of only
err != nil.As written, this test passes for any constructor failure. Checking
errors.Is(err, templig.ErrNoSecretRegexp)keeps it pinned to theWithSecretRE(nil)contract fromconfig.go.Suggested change
_, err := templig.New[TestConfig]( templig.WithFile[TestConfig]("testData/test_config_0.yaml"), templig.WithSecretRE[TestConfig](nil)) - if err == nil { - t.Errorf("setting secret regex to nil should return an error") + if !errors.Is(err, templig.ErrNoSecretRegexp) { + t.Errorf("expected ErrNoSecretRegexp, got %v", err) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config_test.go` around lines 859 - 863, The test around templig.New[TestConfig] should assert the specific sentinel error rather than only checking for a non-nil error. Update the assertion to use errors.Is against templig.ErrNoSecretRegexp so the test stays tied to the WithSecretRE(nil) contract in templig.New/WithSecretRE instead of passing on any constructor failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 10: The changelog entry in CHANGELOG.md uses the wrong template context
name by referencing .Value.valName instead of the actual .Values.valName used by
the template data in config.go; update the documented variable name to match the
plural "Values" so it stays consistent with the README fix and the existing
template symbols.
In `@examples/templating/value/main_test.go`:
- Around line 11-15: The smoke test in TestMainGood only calls main() and never
checks that the example actually loaded, so it can pass even when the
template/render path is broken. Update the test to assert the expected success
path directly, either by invoking templig.New[Config] and checking the result or
by verifying main() produced the expected loaded configuration outcome, so
failures are caught and t is used.
In `@examples/templating/value/main.go`:
- Around line 27-39: The local variable name in the templig example is too short
for the varnamelen lint rule. Rename the `c` variable in the
`templig.New[Config]` assignment to a longer, clearer name such as `cfg`, and
update its subsequent `Get()` and `ToSecretsHiddenStructured()` uses to match
without changing behavior.
In `@examples/templating/value/my_config.yaml`:
- Line 6: The templating example is using the wrong root key, so the value
lookup fails before YAML parsing. Update the expression in the config template
to use .Values.pass instead of .Value.pass, matching the data shape produced by
WithValue and consumed by New so the example renders correctly.
In `@README.md`:
- Line 347: The templating example uses the wrong Helm values reference: replace
`.Value.pass` with `.Values.pass` in the README example to match how `config.go`
exposes custom values. Also search the related templating docs in the CHANGELOG
and `examples/templating/value/*` and update any matching `.Value` usage to
`.Values` so the wording and examples stay consistent.
---
Nitpick comments:
In `@config_test.go`:
- Around line 420-423: The test setup in the templig.New call is not actually
exercising value propagation through overlay because WithValue("does",
"nothing") is unused by both YAML inputs. Update config_test.go so one of the
overlaid sources in the templig.New path references the injected value via
templating, or replace one WithFile source with inline templated YAML, and keep
the relevant symbols templig.New, WithValue, and WithFile in place to verify the
overlay value path.
- Around line 859-863: The test around templig.New[TestConfig] should assert the
specific sentinel error rather than only checking for a non-nil error. Update
the assertion to use errors.Is against templig.ErrNoSecretRegexp so the test
stays tied to the WithSecretRE(nil) contract in templig.New/WithSecretRE instead
of passing on any constructor failure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4d4cfe2-fd67-4c1f-9952-a43f7d4a33cb
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
.golangci.yamlCHANGELOG.mdREADME.mdconfig.goconfig_internal_test.goconfig_test.goexamples/templating/value/main.goexamples/templating/value/main_test.goexamples/templating/value/my_config.yamlgo.modtestData/test_config_3.yaml
…ocumentation accordingly. Signed-off-by: Alexander Adam <alphaone23@gmail.com>
…ingly. Signed-off-by: Alexander Adam <alphaone23@gmail.com>
…roper resource closure. Update tests accordingly. Signed-off-by: Alexander Adam <alphaone23@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@testData/test_config_0_overlay.yaml`:
- Line 6: The YAML overlay entry renders an unquoted template expression, which
can break when the resolved value contains YAML-special characters. Update the
value in test_config_0_overlay.yaml so the .Values.pass2 expression is quoted in
the rendered YAML, keeping the overlay valid for arbitrary inputs and preserving
it as a string.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99b276a0-70ec-43eb-b676-eb6de92dc386
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdconfig.goconfig_internal_test.goconfig_test.goexamples/templating/env/main_test.goexamples/templating/value/main.goexamples/templating/value/main_test.goexamples/templating/value/my_config.yamltestData/test_config_0_overlay.yaml
💤 Files with no reviewable changes (1)
- examples/templating/env/main_test.go
✅ Files skipped from review due to trivial changes (3)
- CHANGELOG.md
- examples/templating/value/my_config.yaml
- README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/templating/value/main.go
- config_internal_test.go
- config_test.go
- config.go
… with YAML best practices. Signed-off-by: Alexander Adam <alphaone23@gmail.com>
2 participants
WithValuefor templated access to custom key-value pairs..Valuevariable for configuration templates.Newfor a unified interface.golang.org/x/toolsto v0.47.0.Summary by CodeRabbit
New[T](...)functional-options API for constructing configs, includingWithFile,WithReader,WithValue, andWithSecretRE..Values.ToFile.SecretREset to nil.