Replace validate_apache_loglevel() with data type #2023
Conversation
alexjfisher
force-pushed
the
replace_validate_apache_log_level_function
branch
from
10c6111 to
ce072b1
Compare
| it { is_expected.to allow_value(allowed_value) } | ||
| end | ||
|
|
||
| [ |
There was a problem hiding this comment.
How about xemerg to test if ^ at the start is needed?
There was a problem hiding this comment.
The regex is copied from the the function and is very lax and could be better. I thought about trying to make it stricter, but decided that a change like that might be suited to a separate PR.
As things stand a ^ at the beginning would be bad. The apache docs describe the allowed syntax as...
Syntax
LogLevel [module:]level [module:level] ...
I think in a perfect world, $log_level would have actually accepted an Array. I doubt it's worth introducing that sort of change now though.
types/loglevel.pp
Outdated
| @@ -0,0 +1,2 @@ | |||
| # @summary As per http://httpd.apache.org/docs/current/mod/core.html#loglevel | |||
There was a problem hiding this comment.
Would @see be useful for the URL?
There was a problem hiding this comment.
I've had a go at making the puppet-strings based docs better. Changes to the REFERENCE.md aren't in this commit as it's very difficult to exclude unrelated changes. It looks like the REFERENCE.md will be updated as part of the release process.
The function was introduced before Puppet 4 was released. It was originally created to reduce duplicated use of stdlib's `validate_re`. See puppetlabs#1097
alexjfisher
force-pushed
the
replace_validate_apache_log_level_function
branch
from
ce072b1 to
426099e
Compare
| @@ -195,8 +195,8 @@ | |||
| # > **Note**: Do not configure this parameter manually without special reason. | |||
| # | |||
| # @param log_level | |||
| # Changes the error log's verbosity. Valid options are: `alert`, `crit`, `debug`, `emerg`, `error`, | |||
| # `info`, `notice` and `warn`. | |||
| # Configures the apache [LogLevel](https://httpd.apache.org/docs/current/mod/core.html#loglevel) directive | |||
There was a problem hiding this comment.
| # Configures the apache [LogLevel](https://httpd.apache.org/docs/current/mod/core.html#loglevel) directive | |
| # Configures the Apache [LogLevel](https://httpd.apache.org/docs/current/mod/core.html#loglevel) directive |
There was a problem hiding this comment.
Thanks for the change @alexjfisher ! LGTM. Had a previous comment about updating the REFERENCE until I realised it would be done as part of our release - so apologies, please ignore that comment if it landed in your inbox.
…apache_log_level_function Replace validate_apache_loglevel() with data type
The function was introduced before Puppet 4 was released. It was
originally created to reduce duplicated use of stdlib's
validate_re.See #1097