fix: function for default config#201
Conversation
anisatahlil
changed the title
| return sanitizedUserName + hash(emailAddress) | ||
| } | ||
|
|
||
| func defaultConfig() Config { |
There was a problem hiding this comment.
This can just be a variable
| @@ -24,13 +24,13 @@ func toYaml(filePath string, configData Config) error { | |||
| } | |||
|
|
|||
| func fromYaml(filePath string) (Config, error) { | |||
There was a problem hiding this comment.
The more I read this the less I like it due to the tight coupling to the Config interface. Ideally, this should use Generics which will be released with Go 1.18 in February in 2022.
If we wanted this to read like Go 1.17 code, we should pass in the interface to "fromYaml" which then passes it through to the yaml.Unmarhsal function. This would allow the fromYaml function to be generic as the function name suggests that it is. It isn't a large change so despite it being pre-optimization this is probably the correct path forward.
There was a problem hiding this comment.
Yeah fromYaml is tightly coupled with the config Interface and generics would help resolve that once that is released. I am okay to pass the interface to set this function on a path to work more generically. But I am also wondering if we have similar functions that can benefit from generics, would need similar changes and if that could perhaps be addressed in a separate PR
There was a problem hiding this comment.
We would have other code like that but lets keep it simple for now and just address this file for now and address the others as they come up.
3 participants
Issue #, if available:
Description of Changes
This is a follow up to address the comments in #197. This change involves adding a function that returns the default config.
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license