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

update the template to declare the input args as a struct type #54

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hcliff
Copy link

@hcliff hcliff commented Apr 13, 2018

instead of redefining a struct repeatedly create a type to declare the input args.

My motivation was to allow me to make assertions on this call in my tests (ginkgo/gomega in the example below) without needing to redeclare the struct myself

Eventually(func() []listservicesstub.PersistListsCall {
  return elasticStub.PersistListsCalls()
}).Should(ConsistOf(listservicesstub.PersistListsCall{
  Lists: []models.List{newList},
}))

100% backwards compatible and actually results in less generated code

@hcliff hcliff changed the title update the template to declare the input args as a struct update the template to declare the input args as a struct type Apr 13, 2018
@hcliff
Copy link
Author

hcliff commented Jul 23, 2018

any thoughts about this? would love to get it merged

calls = mock.calls.{{.Name}}
lock{{$obj.InterfaceName}}Mock{{.Name}}.RUnlock()
return calls
defer lock{{$obj.InterfaceName}}Mock{{.Name}}.RUnlock()
Copy link
Owner

Choose a reason for hiding this comment

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

Could we have this on two lines? I'm not a fan of compressing code onto one line at the cost of readability?

{{- end }}
}
}
{{ range .Methods }}

// {{.Name}}Call represents the input arguments to {{.Name}}
type {{.Name}}Call struct {
Copy link
Owner

Choose a reason for hiding this comment

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

How much noise does this add to the API?

@matryer
Copy link
Owner

matryer commented Feb 2, 2021

Hey @hcliff - thank you so much for this. I wonder if the additional types in the package space end up cluttering it too much? What do you think?

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.

2 participants