-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
migrate away from erb #357
base: master
Are you sure you want to change the base?
Conversation
95232a6
to
f1c8e1b
Compare
# @summary Remove blank lines from a string | ||
# @param content The content to remove blank lines from | ||
# @return The content with blank lines removed | ||
function unbound::clean_blank_lines ( |
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 think this is generic enough that it could also be part of extlib?
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.
$config_file = $unbound::config_file, | ||
Array[Unbound::Resource_record_type] $local_data = [], | ||
String $template_name = 'unbound/local_zone.erb' | ||
Unbound::Local_zone_type $type, |
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 prefer the older style, with just a single whitespace. That makes the diff easier to read and makes git reverts / cherry-picks easier. But there's no consent in our style guide yet.
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 hear you but the rest of this module uses this style and I prefer consistency. But consistency across viz would be great so let's try and get something in the style guide and then we can creat a puppet lint check and fix to get everything fixed?
) { | ||
$records = $local_data.map |$record| { |
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.
there's a lot going on here. Can you please add some comments about what's happening?
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.
Fair comment will do
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.
Updated let me know what's missing?
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.
if you have any idea on the spec test failure please say because im lost
) { | ||
$records = $local_data.map |$record| { |
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.
Updated let me know what's missing?
# @summary Remove blank lines from a string | ||
# @param content The content to remove blank lines from | ||
# @return The content with blank lines removed | ||
function unbound::clean_blank_lines ( |
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.
update the templating code to move away from erb and where possible use the print_config function
This PR is currently a WIP, couple of outstanding things
Also happy to split this up into separate PRs if needed