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

http and ftp proxy variables should be lowercase #123

Merged
merged 1 commit into from
Dec 12, 2016

Conversation

CorCornelisse
Copy link

As per make.conf documentation, the only two variables that are
actually not uppercase, are "http_proxy" and "ftp_proxy.

As per make.conf documentation, the only two variables that are
actually not uppercase, are "http_proxy" and "ftp_proxy.
@@ -1,5 +1,7 @@
<% if @content == '' -%>
<%= @name %>
<% elsif @name == 'http_proxy' or @name == 'ftp_proxy' -%>

Choose a reason for hiding this comment

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

Is it possible to move this input munging to the definition instead of doing it in the template?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not 100% sure if I understand correctly, by "moving it to the definition", you mean moving all of it into makeconf.pp where makeconf is defined? If so, without making this module dependent on stdlib or duplicating code from it, the only way to achieve filtering and changing case is by using inline ruby if I'm not mistaken?
I'm not exactly sure if this results in better readable code. I agree it might not be the best position to do this at template level, but one could argue it's merely ensuring the proper make.conf syntax is generated. Why not do so at the lowest possible level, the template.

Choose a reason for hiding this comment

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

Could inline_template be another way to do it? Anyway, we planned to
convert portage::makeconf into a native provider for more flexibility,
see #56.

@robbat2 robbat2 merged commit 6415937 into gentoo:master Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants