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 custom template option #41

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

dhruvagarwal
Copy link
Contributor

This is a POC for adding a custom template. Currently this is intended to work when no language option is provided .

help='Add the data that you want to send along with the header.')
parser.add_argument(
'--template', '-t',
help='Add the path of the custom template you want to provide.')
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add proper name for the custom template option ;)

@DePierre
Copy link
Contributor

@dhruvagarwal I am not really fan of this idea for 2 reasons:

  • If the user wants to use a custom template, s-he can write it under './templates/'
  • If the user has to write the template under './templates/', it would increase the chances for them to create a PR in order to add their templates in the repo (imo).

What do you think? @arunk-s any thoughts on that?

@arunk-s
Copy link
Contributor

arunk-s commented Mar 13, 2016

@DePierre I think your idea of writing the templates under a global directory sounds awesome, it will help us further in discovering the ways the users are using our tool, may be discover some bugs in our templates/code may be.

@DePierre
Copy link
Contributor

@dhruvagarwal Do you think that you could add unittests for that new feature as well as the source code? That will be very helpful before merging the changes :)

help='Add the data that you want to send along with the header.')
parser.add_argument(
'--custom-template', '-t',
help='Add the path of the custom template you want to provide.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid 'you'. Keep it broad like "Specify path for custom templates."

@DePierre
Copy link
Contributor

@dhruvagarwal Reading the modification, what happens if the user specifies a custom template bash.py? Is it overidden by the default one http_request_translator provides?

@DePierre
Copy link
Contributor

DePierre commented Apr 6, 2016

@dhruvagarwal Do you need more information in order to update your PR regarding the custom template idea?

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

Successfully merging this pull request may close these issues.

3 participants