-
Notifications
You must be signed in to change notification settings - Fork 719
feat: squash with prefix #291
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| // Make *T | ||
| copy := reflect.New(elem.Type()) | ||
| clone := reflect.New(elem.Type()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prevent shadowing of built-in function copy
|
|
||
| // Accumulate errors | ||
| errors := make([]string, 0) | ||
| errs := make([]string, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prevent shadowing of errors package
| } | ||
|
|
||
| func (d *Decoder) decodeMapFromStruct(name string, dataVal reflect.Value, val reflect.Value, valMap reflect.Value) error { | ||
| func (d *Decoder) decodeMapFromStruct(_ string, dataVal reflect.Value, val reflect.Value, valMap reflect.Value) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused name argument
|
@mitchellh, hi! I'm sorry for the ping. Is there a chance to get feedback here? |
| if prefix == "" { | ||
| return s | ||
| } | ||
| return prefix + "_" + s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add the _? Wouldn't it be more flexible to just add it to the prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be more flexible if added to DecoderConfig. I preferred to make it simple.
Also, for me, it's just the same approach as https://cs.opensource.google/go/go/+/refs/tags/go1.21.3:src/path/path.go;l=162-180
|
Any chances on getting this merged in the foreseeable feature? I'm also highly looking forward to this feature! Thank you, @kamilsk, for submitting this! |
| } | ||
|
|
||
| type GitHub struct { | ||
| Git `mapstructure:"git,squash"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems not possible to use another name than git here. For example if you rename this to
| Git `mapstructure:"git,squash"` | |
| Git `mapstructure:"gitx,squash"` |
and update the input below accordingly (to GITHUB_GITX_REMOTE), the test panics.
Is this the desired behavior? I would have expected to be able to use any name here instead of just the name of the embedded struct.
Fix issue #288.
Temporary: how to support this feature in your env
Tested with
viperandpflaghttps://github.com/octomation/maintainer/blob/dbfaab2b34ba9d2363d3e665b20c8bb5783581b1/internal/config/tool_test.go#L40-L87