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 sort to LogFormats to ensure consistency between runs #829

Merged
merged 1 commit into from
Nov 5, 2014

Conversation

tjikkun
Copy link
Contributor

@tjikkun tjikkun commented Aug 25, 2014

No description provided.

@@ -56,7 +56,7 @@ LogFormat "%h %l %u %t \"%r\" %>s %b" common
LogFormat "%{Referer}i -> %U" referer
LogFormat "%{User-agent}i" agent
<% if @log_formats and !@log_formats.empty? -%>
<%- @log_formats.each do |nickname,format| -%>
<%- @log_formats.sort.each do |nickname,format| -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

you can contract those two lines of if / do into:

<% Hash(@log_formats).sort.each do |nickname, format| -%>

@tjikkun
Copy link
Contributor Author

tjikkun commented Sep 9, 2014

@igalic I changed it liked you asked to, but is it possible this is a Ruby >= 2.0 only thing?

@qha
Copy link

qha commented Oct 14, 2014

@tjikkun If your question concerns the need for sort then no, i have the same problem on "ruby 1.8.7 (2011-06-30 patchlevel 352) [x86_64-linux]". If your question concerns the patch then possibly, i really don't know enough Ruby to answer that.

@tjikkun
Copy link
Contributor Author

tjikkun commented Oct 14, 2014

@igalic is "<% Hash(@log_formats).sort.each do |nickname, format| -%>" a Ruby >= 2.0 only thing?
It doesn't pass the unit tests for older versions. Can you tell me what you want me to do? Revert to my first patch, something else?

@underscorgan
Copy link
Contributor

@tjikkun yeah, that seems to only work on 2.0. Can you go back to your initial change? Sorry for the confusion.

@tjikkun
Copy link
Contributor Author

tjikkun commented Nov 5, 2014

@mhaskel ok, reverted back to initial patch. Hope it can be merged soon.

underscorgan pushed a commit that referenced this pull request Nov 5, 2014
add sort to LogFormats to ensure consistency between runs
@underscorgan underscorgan merged commit 23a2dde into puppetlabs:master Nov 5, 2014
@underscorgan
Copy link
Contributor

Thanks for the contribution @tjikkun

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

Successfully merging this pull request may close these issues.

None yet

5 participants