-
Notifications
You must be signed in to change notification settings - Fork 449
feat: goimports local prefix #1138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3
Are you sure you want to change the base?
Conversation
5b14a70 to
bc0ef83
Compare
bc0ef83 to
dc7ca2f
Compare
LandonTClipp
left a comment
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.
Looks great, let's just extend this a bit more.
config/config.go
Outdated
| Recursive *bool `koanf:"recursive" yaml:"recursive,omitempty"` | ||
| ForceFileWrite *bool `koanf:"force-file-write" yaml:"force-file-write,omitempty"` | ||
| Formatter *string `koanf:"formatter" yaml:"formatter,omitempty"` | ||
| FormatterOptions FormatterOptions `koanf:"formatter-options" yaml:"formatter-options,omitempty"` |
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.
This should be *FormatterOptions so that the yaml map can be nil.
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.
Wouldn't we just end up with a zero value FormatterOptions in that case? I was thinking to do it this way to avoid the nil checks later, might be missing something though.
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.
The reason is pretty complicated. It has to do with the migrate command when going from v2 to v3. If you set the default in koanf then mockery will never have nil values for it.
internal/template_generator.go
Outdated
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.
This is good but I think people also want to be able to modify the imports.Options struct. I think we can do a functional option style of thing. I suggest something like this:
type FormatterOptions struct {
GoImports *GoImports
}
type GoImports struct {
LocalPrefix *string ...
TabWidth *int ...
}
func (g GoImports) Options() imports.Options {
return imports.Options{
TabWidth: g.TabWidth,
// etc
}
}Then:
func goimports(src []byte, opt GoImports) ([]byte, error) {
imports.LocalPrefix = opt.LocalPrefix
formatted, err := imports.Process("/", src, opt.Options())We'd want to set default values in here: https://github.com/rfwatson/mockery/blob/dc7ca2f8c944d58757e85aef489fee65a5da13fd/config/config.go#L68-L85
Description
Add goimports local-prefix configuration. Fixes #1098.
Type of change
Version of Go used when building/testing:
How Has This Been Tested?
Mostly manual testing. It seems that testing the actual formatter call could be a bit tricky, happy to take a look at adding some if you think it's worth it though.
Checklist