Bump minimum stdlib version to be 4.25.0 #193
Conversation
* bump version-requirement for stdlib to 4.25.0 (needed for Stdlib::IP::*) * further enforce serveraddress in sql to be an ip-address and no subnet * remove unnecessary Optional[] for already defined valued
|
Added suggestions from #192 BTW: looking at the code there are some more Optionals with predefined values, maybe those can be cleaned up too, but I guess those are better in a separate PR. |
|
@nward This looks ok to me by eye but I have no way of testing it. Are you able to review & merge? I think we're getting to a stage where we should do a new release, so I want to get as many PRs merged as possible |
There was a problem hiding this comment.
Given that we have already merged #192, I think the only one we need to address is the minimum version of stdlib.
We need to do a big clean up of the Optionals and other type things - so I think we should do those all at once.
There is no problem with these changes being done here - but I think we should try keep PRs to one issue at a time.
manifests/module/detail.pp
Outdated
| @@ -9,7 +9,7 @@ | |||
| String $header = '%t', | |||
| Optional[Freeradius::Boolean] $locking = undef, | |||
| Optional[Freeradius::Boolean] $log_packet_header = undef, | |||
| Optional[Array[String]] $suppress = [], | |||
| Array[String] $suppress = [], | |||
There was a problem hiding this comment.
| Array[String] $suppress = [], | |
| Optional[Array[String]] $suppress = [], |
manifests/sql.pp
Outdated
| @@ -2,7 +2,7 @@ | |||
| define freeradius::sql ( | |||
| Enum['mysql', 'mssql', 'oracle', 'postgresql'] $database, | |||
| Freeradius::Password $password, | |||
| Variant[Stdlib::Host, Stdlib::IP::Address] $server = 'localhost', | |||
| Variant[Stdlib::Host, Stdlib::IP::Address::Nosubnet] $server = 'localhost', | |||
There was a problem hiding this comment.
| Variant[Stdlib::Host, Stdlib::IP::Address::Nosubnet] $server = 'localhost', | |
| Variant[Stdlib::Host, Stdlib::IP::Address] $server = 'localhost', |
(actually it turns out that this can just be Stdlib::Host - as Host already covers single IP addresses which is nice - but I don't think we should make that change in this PR)
nward
changed the title
bump version-requirement for stdlib to 4.25.0 (needed for Stdlib::IP::*)
further enforce serveraddress in sql to be an ip-address and no subnet
remove unnecessary Optional[] for already defined valued