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

Remove spurious ':' from refresh_pattern template #87

Merged
merged 1 commit into from
Mar 27, 2018

Conversation

ralfbosz
Copy link
Contributor

When using a regexp (when case_sensitive is set to false)
the -i should be placed BEFORE the $name and not after
Also fixed that no space was used between the max and
options field, added rspec for options

@ralfbosz ralfbosz force-pushed the fix/refresh_pattern branch from 03fdd5d to 4001565 Compare February 20, 2018 07:28
@ralfbosz
Copy link
Contributor Author

in doubt about the : which is used in the template, that should not be fixed, only few refresh_patterns use it, gonna change that too... after some testing...

When using a regexp (when case_sensitive is set to false)
the -i should be placed BEFORE the $name and not after
Also fixed that no space was used between the max and
options field, added rspec for options
Removed default : which was used, this is not default,
changed README so ^ftp: has it mentioned
@ralfbosz ralfbosz force-pushed the fix/refresh_pattern branch from 4001565 to 4cdfe85 Compare February 20, 2018 08:16
@ralfbosz
Copy link
Contributor Author

remove the default use of : after the title, this is not default, see examples:

http://www.squid-cache.org/Doc/config/refresh_pattern/

@ralfbosz
Copy link
Contributor Author

seems I didn't check all the PR's, this is in part: #65

@bastelfreak bastelfreak requested a review from traylenator March 27, 2018 09:12
@traylenator traylenator added the bug Something isn't working label Mar 27, 2018
@traylenator
Copy link
Contributor

traylenator commented Mar 27, 2018

To be clear someone doing.

squid::refresh_pattern{'ftp':
  min  => 20,
  max => 30,
  percent 100,
}

were getting

refresh_pattern ftp: 100 20 30

and now the will get

refresh_pattern ftp 100 20 30

backwards incompatible but the old behaviour was wrong.

@ralfbosz
Copy link
Contributor Author

ralfbosz commented Mar 27, 2018

Correct, the old behaviour was wrong, see also #65 which also addresses this, but fails in Travis...

For you sample, the person would have to use:

squid::refresh_pattern{'ftp:':
  min => 20,
  max => 30,
  percent => 100,
}

@traylenator traylenator changed the title This commit fixes the refresh_pattern template Remove spurios ':' from refresh_pattern template Mar 27, 2018
@traylenator traylenator changed the title Remove spurios ':' from refresh_pattern template Remove spurious ':' from refresh_pattern template Mar 27, 2018
@traylenator traylenator merged commit 3aa5b12 into voxpupuli:master Mar 27, 2018
Copy link
Contributor

@traylenator traylenator left a comment

Choose a reason for hiding this comment

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

This all make sense. The : should not be there.

@ralfbosz ralfbosz deleted the fix/refresh_pattern branch March 27, 2018 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatible bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants