Skip to content
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

Add feature to generate mocks in directory #193

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

horpto
Copy link

@horpto horpto commented Mar 27, 2023

Hello, we use moq to generate mocks in the different directory in 1 mock to 1 file. So our genmock.go has a pile lines like this:

//go:generate moq -out "mocks/fee.go" -fmt goimports -skip-ensure . IFeeInterface
//go:generate moq -out "mocks/foo.go" -fmt goimports -skip-ensure . IFooInterface
//go:generate moq -out "mocks/goo.go" -fmt goimports -skip-ensure . IGooInterface

I'd like to add a feature to just specify directory and list of interfaces in one package. This also should be more quickly to generate mocks.

Also I fix version indication in release as gorelease will specify main.version, not main.Version

Copy link
Contributor

@breml breml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @horpto

Thanks for your contribution. I skimmed through your PR and some thoughts come to mind, that I would like to share with you.

General:

  • For such an extension, it would have been beneficial to first create an issue, such that the topic can be discussed and multiple different approached could have been evaluated.
  • Maybe the fix for the version should be in a separate PR, since this is pretty likely to be accepted and merged quickly, instead of combining this with a PR, where there is not yet any guarantee, that it will get accepted in due time.

Specific for the PR:

  • The PR contains special handling for interface names, starting with I. To my knowledge, this is a pattern, that is not normally used in Go code (I would even go so far, that it is discouraged) and therefore I wonder, if a tool like moq should provide support for this.
  • For this PR to be merged, the README.md should be updated as well and reflect the changes to the flags and the arguments. Specifically it should mention, which flags are mutual exclusive.

@@ -30,6 +31,7 @@ type userFlags struct {
func main() {
var flags userFlags
flag.StringVar(&flags.outFile, "out", "", "output file (default stdout)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the -out flag mention as well, that it is exclusive with -out-dir?

main.go Outdated
if err = m.Mock(out, args...); err != nil {
switch {
case flags.outDir != "" && flags.outFile != "":
return errors.New("use only one from -out and -out-dir arguments")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return errors.New("use only one from -out and -out-dir arguments")
return errors.New("use only one of the flags -out and -out-dir")

@horpto
Copy link
Author

horpto commented Mar 28, 2023

Hi @breml! Ok, I created a new pr with only small changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants