-
Notifications
You must be signed in to change notification settings - Fork 137
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
Added support for defining custom provisioners in profile #73
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea and I especially like the fact that you've put it in profiles
and not in the configuration.
👍 pending the minor indentation changes.
|
||
{% if ida_path %}, | ||
{% include 'snippets/ida_remote_64.json' %}, | ||
{% include 'snippets/ida_remote_32.json' %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
{% endif %} | ||
{% if packer_extra_provisioners %} | ||
{% for p in packer_extra_provisioners %} | ||
,{{ p | tojson }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
|
||
{% if packer_extra_provisioners %} | ||
{% for p in packer_extra_provisioners %} | ||
,{{ p | tojson }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
|
||
{% if packer_extra_provisioners %} | ||
{% for p in packer_extra_provisioners %} | ||
,{{ p | tojson }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please indent
|
||
{% if packer_extra_provisioners %} | ||
{% for p in packer_extra_provisioners %} | ||
,{{ p | tojson }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please indent this line (it's under the for loop)
Signed-off-by: Camille Moncelier <[email protected]>
@@ -31,10 +31,14 @@ | |||
{% if tools_path %}, | |||
{% include 'snippets/tools.json' %} | |||
{% endif %} | |||
{% if ida_path %}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines got removed to fix the space/tab indent mixing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I noticed that. Thanks! My vim config is still badly configured for javascript/json, at some point I'll rewrite everything into 4 spaces but not now.
This should be ok now :) |
I'm ok with your changes. I'll ask for a review from @Svieg since this affects users and future direction. If he doesn't comment in 1-2 days I'll merge. |
I think it's a cool idea so look at my comment and I'd be happy to merge that :) |
@@ -487,6 +487,11 @@ def prepare_profile(template, config): | |||
for package_mod in profile["package"]: | |||
package(profile_name, package_mod["package"], fd) | |||
|
|||
if "packer" in profile: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you plan to add other fields to the packer
attribute? For example, If there's only provisionners
, I'd delete packer
and make it clearer IMO. Also, if you plan to add other fields other than provisionners
, what would they be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept in mind the possibility because I might plan on adding support for post-processors (mostly shell like this in order to allow some workflow processinng after the box generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept in mind the possibility because I might plan on adding support for post-processors (mostly shell1 like this in order to allow some workflow processinng after the box generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, I need to fix a missing comma in default-profile.js, right under comment, I on the train right now.
Crap. Too late, I was missing coverage in the train. default-profile.js is lacking a comma right under |
No worries. It's fixed in master. See f086475. |
I made the packer provisioning area customizable using the profile.
I added a packer section:
Which is copied at the end of the packer.json templates.
It allows users to add custom powershell commands (or even scripts) directly.
We use this to push custom packages (using a custom repository) using chocolatey.