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

Support external fake templates #98

Open
joefitzgerald opened this issue Oct 17, 2018 · 7 comments
Open

Support external fake templates #98

joefitzgerald opened this issue Oct 17, 2018 · 7 comments

Comments

@joefitzgerald
Copy link
Collaborator

Allow a user of counterfeiter to supply a path to their own fake template (based on text/template syntax) for use when generating a fake.

This will allow users to:

  • Supply their own copyright/license information in the header
  • Have fakes with slightly altered behavior relative to the default template
  • Try building entirely new templates with different styles (e.g. gomock / Mockito / others...)

Risks associated with this are:

  • The internal data model used today with the fake template becomes part of the public API
  • We could fix bugs in the default template that are not necessarily evident to a user who is using a slightly tweaked version (e.g. with copyright information in the header)
  • Users may come up with innovative/novel templates that are not known to other users (e.g. should we have a template zoo that is hosted in this repository; if so what are the implications to the API if a template has a breaking change?)
@hoegaarden
Copy link
Contributor

hoegaarden commented Oct 18, 2018

A different approach could be to introduce the notion of a header. By default it should be // Code generated by counterfeiter. DO NOT EDIT.. But users can override that. It should not be interpreted as a template but should be a literal string.

This would fit my current use-case where I want to use counterfeiter in kubernetes repos and have to prepend generated files with this header. I could then potentially use counterfeiter with go generate like so:

//go:generate counterfeiter -header-file $KUBE_ROOT/hack/boilerplate/boilerplate.generatego.txt . TheInterfaceToFake

Pros:

  • Allow users to only set headers without need to care about the template itself & therefor reduces the risk for users to break the actual template when all they want to do is just change / extend the header
  • Removes the need to extract the default template(s) from counterfeiter, extend those with the header and commit all the templates (interface, function, package) to the project's repo for later use
  • No need to point counterfeiter to different templates when using it to fake different things (interface or function or package), so easier to use e.g. like that
    //go:generate -command fake_it counterfeiter -header-file $KUBE_ROOT/hack/boilerplate/boilerplate.generatego.txt
    //go:generate fake_it . TheInterfaceToFake
    //go:generate fake_it . TheFunctionToFake

Cons / Risks:

  • Changes to UX (additional cmd line arg like -header-file, some magic env variable like $COUNTERFEITER_HEADER_FILE, ...)
  • Maybe there are cases where it would be great to have the header be a template with some functions available (e.g. have a function to get the current year in the template)

@LasTshaMAN
Copy link

Do you REALLY need this feature ?

Additional handles/toggles make a package harder to understand, lead to documentation bloat.

What would be real world use-cases for such functionality ?

@hoegaarden
Copy link
Contributor

Well, I don't REALLY need it -- I can always slap some sh around counterfeiter which handles that.

However I believe that this usecase (having a licence header or such on all files) is a common enough scenario that counterfeiter authors could think about directly supporting. I can totally see the benefits of counterfeiter providing such a feature (see the real life k8s usecase above -- I'd love to use that without needing a wrapper around counterfeiter), but I can also understand if you all think that's not counterfeiter's responsibility and can see the added complexity you'd need to support.

@LasTshaMAN
Copy link

LasTshaMAN commented Jun 9, 2019

@hoegaarden, about licence header for generated code ... I see your point, but I personaly think (feel free to persuade me otherwise :) generated code shouldn't be checked into git repo ... (rendering this use-case somewhat irrelevant)

@joefitzgerald
Copy link
Collaborator Author

@LasTshaMAN I would make a somewhat educated guess that the vast majority of counterfeiter users commit generated code to a git repo.

In general I do think we should find a way to achieve the outcome (the ability to set a custom header) that @hoegaarden (btw, such a great beer 🍺) is looking for.

@LasTshaMAN
Copy link

I would make a somewhat educated guess that the vast majority of counterfeiter users commit generated code to a git repo.

Why does this happen ? Do you think encouraging it (by adding features like this one) is a good idea ?

@hoegaarden
Copy link
Contributor

I do prefer committing generated code for the most part, because it allows users to clone the repo and just run the thing (where in case of counterfeiter "the thing" are the tests, but you could also have generated production code). They don't need to know how to run the code generator or even have the code generator installed.

When it comes to drift between the fake and the thing to be faked I feel counterfeiter protects that good enough by having the compile time assertion which checks if the fake actually implements the thing it is supposed to stand in for.

I think counterfeiter does not have to impose one or the other way (you must or you must not commit the generated fake) and adding the feature to allow changing/adding a header does not really encourage one or the other from my point of view.

Those are just my 0.02€ and still it is up to the counterfeiter maintainers to state if they'd support something like that or rather not. When it comes to the implementation I would love to spend some cycles on that ... but of course only if there is a chance that the PR will actually be accepted.

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

No branches or pull requests

3 participants