Start hiding viper in public APIs #2806
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2806 +/- ##
==========================================
+ Coverage 91.64% 91.67% +0.02%
==========================================
Files 294 294
Lines 15637 15640 +3
==========================================
+ Hits 14331 14338 +7
+ Misses 896 891 -5
- Partials 410 411 +1
Continue to review full report at Codecov.
|
config/configparser/config.go
Outdated
| v *viper.Viper, | ||
| factories component.Factories, | ||
| ) (*configmodels.Config, error) { | ||
| func Parse(v *config.Parser, factories component.Factories) (*configmodels.Config, error) { |
There was a problem hiding this comment.
Nit:
| func Parse(v *config.Parser, factories component.Factories) (*configmodels.Config, error) { | |
| func Parse(p *config.Parser, factories component.Factories) (*configmodels.Config, error) { |
There was a problem hiding this comment.
Nit: names are a bit confusing. If this is Parse func, then why is it accepting a Parser? Is Parser doing parsing or this function is doing parsing?
Maybe this package should be named configloader instead of configparser and the function is Load instead of Parse?
There was a problem hiding this comment.
Reverted to Load. Will update the package later.
| @@ -120,33 +116,33 @@ func Load( | |||
|
|
|||
| // Start with the service extensions. | |||
|
|
|||
| extensions, err := loadExtensions(v.GetStringMap(extensionsKeyName), factories.Extensions) | |||
| extensions, err := loadExtensions(cast.ToStringMap(v.Get(extensionsKeyName)), factories.Extensions) | |||
There was a problem hiding this comment.
What is the final destination? Are we going to get rid of cast.ToStringMap?
There was a problem hiding this comment.
I don't think I need it, but will followup. I want to start with a minimal API for Parser
The new Parser struct is mostly a wrapper around Viper with bunch of fixes. Signed-off-by: Bogdan Drutu <[email protected]>
The new Parser struct is mostly a wrapper around Viper with bunch of fixes.
Signed-off-by: Bogdan Drutu [email protected]