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/Fix data types of all parameters #634

Merged
merged 8 commits into from
Aug 19, 2021
Merged

Add/Fix data types of all parameters #634

merged 8 commits into from
Aug 19, 2021

Conversation

smortex
Copy link
Member

@smortex smortex commented Aug 19, 2021

Pull Request (PR) description

No functional change. Fixes for invalid/wrong/weird data types will be part of another PR.

The initial intention was to have a backward compatible PR (this one) and one that fixes the wrong types, but because the code is inconsistent, it is not possible to add data types in a backward compatible way.

This Pull Request (PR) fixes the following issues

n/a

@smortex smortex added the enhancement New feature or request label Aug 19, 2021
manifests/params.pp Outdated Show resolved Hide resolved
We can make the code more expressive and shorted by using a proper data
type.
@smortex smortex changed the title Add data types to all parameters Add/Fix data types of all parameters Aug 19, 2021
These parameters had mixed types before data-types where added.  Use the
more appropriate data type for each of them.
Before we got data types, $security_limit_extensions was an optional
array of string defaulting to undef.  It is now an array of string
defaulting to an empty array.

We do not want to emit security.limit_extensions when this array is
empty.
@smortex smortex marked this pull request as ready for review August 19, 2021 02:53
@smortex
Copy link
Member Author

smortex commented Aug 19, 2021

So, I do not know how much we can trust the test suite 🤡… To be sure I applied the changes on our production infrastructure and it seems to be fine 😄 (I only saw changes from PR which have already been merged into the master branch).

Ready for review!

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

Successfully merging this pull request may close these issues.

2 participants