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

Fix $openldap::server::database data type #329

Merged
merged 1 commit into from
Sep 22, 2021
Merged

Conversation

smortex
Copy link
Member

@smortex smortex commented Sep 22, 2021

Pull Request (PR) description

It is expected to be a boolean value. Was typed as string by mistake in
c2ea6cb.

This Pull Request (PR) fixes the following issues

Fixes #328

@smortex smortex added the bug Something isn't working label Sep 22, 2021
@smortex smortex changed the title Fix openldap::server::database data type Fix $openldap::server::database data type Sep 22, 2021
@smortex smortex marked this pull request as ready for review September 22, 2021 19:10
@@ -7,7 +7,7 @@
Optional[String[1]] $backend = undef,
Optional[String[1]] $rootdn = undef,
Optional[String[1]] $rootpw = undef,
Optional[String[1]] $initdb = undef,
Optional[Boolean] $initdb = undef,
Copy link
Member

Choose a reason for hiding this comment

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

I find Optional strange for Booleans, because isn't undef equivalent to false? Seems like the default is true in the type. Maybe remove Optional and default it to true?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is indeed over-complicated and I was trying to decipher this.
Defaulting to true should be the right thing to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to reverse this because the default value actually depend on the backend.

It is expected to be a boolean value.  Was typed as string by mistake in
c2ea6cb.

While here, fix the parameter description.
@smortex
Copy link
Member Author

smortex commented Sep 22, 2021

Okay, to quote a comment once read on a mailing list, "Don't trust the comments!" :-) The parameter defaults to true… unless we are in the cases where it default to false 🙃

At least, we have a CI that helped reading the right information in the right place!

Reverted and updated the doc to match the code.

@smortex smortex merged commit ae8fba4 into master Sep 22, 2021
@smortex smortex deleted the initdb-data-type branch September 22, 2021 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openldap::server::database initdb data type should support value false
3 participants