[ENHANCEMENT] Struct support #2
Conversation
|
Thanks for opening this PR! I think supporting nested structs is a great feature. Some early comments:
Thanks again! |
|
Hi, Thanks for your comment. I have added these things. |
| value, ok := params[field.name] | ||
| if !ok { | ||
| value = field.defaultValue | ||
| func buildStructSpec(configPath string, t reflect.Type, spec *map[string]fieldSpec) { |
There was a problem hiding this comment.
Is there any reason this returns a pointer to a map? A map is already a pointer-ish type
| func buildStructSpec(configPath string, t reflect.Type, spec *map[string]fieldSpec) { | |
| func buildStructSpec(configPath string, t reflect.Type, spec map[string]fieldSpec) { |
| "github.com/aws/aws-sdk-go/aws" | ||
| "github.com/aws/aws-sdk-go/aws/session" | ||
| "github.com/aws/aws-sdk-go/service/ssm" | ||
| "github.com/aws/aws-sdk-go/service/ssm/ssmiface" | ||
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| type ( |
There was a problem hiding this comment.
Please move these type declarations back to their original places in the file. Making stylistic changes pollutes the diff.
| @@ -48,59 +54,56 @@ type Provider struct { | |||
| // The behavior of using the `default` and `required` tags on the same struct field is | |||
| // currently undefined. | |||
| func (p *Provider) Process(configPath string, c interface{}) error { | |||
|
|
|||
There was a problem hiding this comment.
Please remove all of the unnecessary white space. Again, this pollutes the diff.
| @@ -40,6 +46,7 @@ func TestProvider_Process(t *testing.T) { | |||
| F641 float64 `ssm:"/float64/f641"` | |||
| F642 float64 `ssm:"/float64/f642" default:"42.42"` | |||
| Invalid string | |||
| D d | |||
There was a problem hiding this comment.
Please also add a pointer to D to ensure pointers to structs also populate as expected.
var s struct {
...
D1 d
D2 *d
}| @@ -48,59 +54,56 @@ type Provider struct { | |||
| // The behavior of using the `default` and `required` tags on the same struct field is | |||
| // currently undefined. | |||
| func (p *Provider) Process(configPath string, c interface{}) error { | |||
There was a problem hiding this comment.
Please add documentation around the behavior of nested structs in this comment and in the readme.
No description provided.