refactor(config): replace manual viper mapping with Unmarshal#870
refactor(config): replace manual viper mapping with Unmarshal#870
Conversation
- Replace 84 manual viper.GetX() calls with viper.Unmarshal() using yaml struct tags - Remove 116-line defaultConf YAML blob, consolidate all defaults into setDefaults() - Use viper.New() instead of global singleton to prevent test pollution - Extract shared test assertions into assertCommonConfig helper - Remove tests that referenced deleted defaultConf variable - Fix Huawei test that relied on placeholder default values Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors configuration loading to reduce manual viper key mapping by unmarshalling directly into the existing ConfYaml struct, consolidating defaults into setDefaults(), and reducing test duplication.
Changes:
- Replace extensive
viper.GetX()field-by-field mapping withviper.Unmarshal()(using existingyamltags) and a small post-processing step forFeedbackHeader. - Remove the embedded default YAML blob and rely on a single defaults source (
setDefaults(v)), using a per-callviper.New()instance to avoid global state pollution. - Refactor config tests to share common assertions via an
assertCommonConfighelper and adjust Huawei HMS test setup.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
config/config.go |
Switch to instance-based viper + Unmarshal and consolidate config defaults in one place. |
config/config_test.go |
Remove tests tied to the removed default YAML blob; dedupe shared assertions via a helper. |
go.mod |
Promote github.com/go-viper/mapstructure/v2 to a direct dependency for decoder config customization. |
notify/notification_hms_test.go |
Ensure TestMissingHuaweiAppID fails for the intended missing AppID case by setting a secret. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix misleading comment about config file search behavior - Return error on config parse/permission failures instead of silently falling back to defaults; only ignore ConfigFileNotFoundError Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
viper.GetX()calls inloadConfigFromViper()with a singleviper.Unmarshal()call using existingyamlstruct tagsdefaultConfYAML blob that duplicatedsetDefaults(), consolidating into a single source of truth for all config defaultsviper.New()instead of the global viper singleton to prevent test pollution across 65+LoadConf()callsTestValidateConfDefaultandTestValidateConfinto anassertCommonConfighelperTest plan
go test -v -tags sqlite ./config/...— all config tests passgo test -tags sqlite ./app/... ./core/... ./logx/... ./metric/... ./rpc/...— all non-infra tests passgo test -tags sqlite -run TestMissing ./notify/...— Huawei tests pass with fixed assertionsmake lint— 0 issues🤖 Generated with Claude Code