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

Contributed templates #1625

Closed
michaelf-stratoscale opened this issue Jul 19, 2018 · 3 comments
Closed

Contributed templates #1625

michaelf-stratoscale opened this issue Jul 19, 2018 · 3 comments

Comments

@michaelf-stratoscale
Copy link
Contributor

Add new feature to support contributed templates.
As a result of https://github.com/Stratoscale/swagger and after discussion with @fredbi we would like to add an option for defining "contributed" templates which will be delivered with the swagger binary.

The suggested solution is to add a --template flag to the generate command, and specify the contributed template name. This will result in overriding the default templates from templates which are included in the binary.

This solution also need to give a way for custom templates to control the generated options, such as excluding main or rewriting the configureapi file.

@fredbi
Copy link
Contributor

fredbi commented Jul 19, 2018

Hello Michael and thank you for willing to contribute.

A few remarks:

  • --template should be more explicit about the intent. I suggest --use-templates
  • this option should not be a flag but a multi valued option - the value default takes the current set of templates
  • contributed templates do not tamper with locked templates
  • this option should only be available to generate client and generate server commands

which will be delivered with the swagger binary.

I am not sure about this one. Should be discussed with @casualjim.

Best of course would be to provide a plugin mechanism, but we are a bit far away from this.
Plugin could be envisioned as a future enhancement.

This solution also need to give a way for custom templates to control the generated options,
such as excluding main or rewriting the configureapi file.

Defaults settings for alternate template sets should be handled in the corresponding ./cmd/generate.
If you propose a set of new generation options (e.g. --use-templates-options) , it would be adequate to handle them in the way we handle language options in the GenOpts structure.

@posener
Copy link
Contributor

posener commented Jul 19, 2018

  • --template should be more explicit about the intent. I suggest --use-templates

Why plural?

  • this option should not be a flag but a multi valued option - the value default takes the current set of templates

We don't really understand what you mean here.
As was implemented in #1626 - If no flag is specified, the default templates are loaded. If one wants to use a template from "contrib", it can specify it with the --template or --use-template flag. If one will use --use-template stratoscale it will use all stratoscale templates override.

I am not sure about this one. Should be discussed with @casualjim.

This is what we discussed in the Slack as the current solution, if there is an alternative in mind, we are willing to implement it.

Defaults settings for alternate template sets should be handled in the corresponding ./cmd/generate.
If you propose a set of new generation options (e.g. --use-templates-options) , it would be adequate to handle them in the way we handle language options in the GenOpts structure.

This is more or less what was implemented in #1626 , right?

@fredbi
Copy link
Contributor

fredbi commented Jul 19, 2018

Hello @posener

Plural because I think that there are more than one way to produce supplementary sets of templates, and moreover, you provide more than one template. It is actually a whole set of new templates.

For instance, I might contribute just like you my own templates to generate tests for operations.
It is not just a go default or go strato choice.

What I mean is something like --use-templates=stratoscale.
So you may well specify and support one single value here (yours) but this is not a flag (ie true|false). So it remains extensible.

This is more or less what was implemented in #1626 , right?

Yep. I wrote this comment before I reviewed the PR. I made some comments about some details, but I agree with the principle.

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