Skip to content

Improve datatypes#219

Merged
bastelfreak merged 1 commit intovoxpupuli:masterfrom
bastelfreak:types
Mar 9, 2019
Merged

Improve datatypes#219
bastelfreak merged 1 commit intovoxpupuli:masterfrom
bastelfreak:types

Conversation

@bastelfreak
Copy link
Copy Markdown
Member

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

@bastelfreak bastelfreak added the enhancement New feature or request label Mar 9, 2019
Copy link
Copy Markdown
Member

@alexjfisher alexjfisher left a comment

Choose a reason for hiding this comment

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

Looks mostly correct. Dns search domains is probably the only thing that needs to be changed.

Comment thread manifests/init.pp Outdated
Stdlib::Port $ldap_port = 389,
String[1] $ldap_server = 'localhost',
String[1] $ldap_username = 'cn=root, dc=example, dc=com',
String[0] $ldap_password = '',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the default is an empty string and I didn't want to change it. Personally I wuld perfer undef. Do you want me to change it? That would be a breaking change (next release already is a major one, so we could include it).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant why not leave it as String

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah yes. doesn't make a difference

Comment thread manifests/init.pp Outdated
Comment thread manifests/init.pp Outdated
Array[Stdlib::Fqdn] $dnssearchdomains = [],
String[1] $dhcp_conf_header = 'INTERNAL_TEMPLATE',
String[1] $dhcp_conf_ddns = 'INTERNAL_TEMPLATE',
String[1] $dhcp_conf_ntp = 'INTERNAL_TEMPLATE',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are empty strings definitely not allowed here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

allowed empty string again

Comment thread manifests/init.pp
Enum['allow', 'deny'] $ddns_client_updates = 'allow',
Optional[Stdlib::IP::Address] $pxeserver = undef,
Optional[String[1]] $pxefilename = undef,
Optional[Integer[1]] $mtu = undef,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An MTU of 1 is no more valid than 0

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure which is the lowest positive number that makes sense, I just wanted to prohibit negative numbers.

Comment thread manifests/init.pp Outdated
@bastelfreak bastelfreak force-pushed the types branch 3 times, most recently from 1be5691 to 44b810e Compare March 9, 2019 08:50
@alexjfisher alexjfisher changed the title improve datatypes Improve datatypes Mar 9, 2019
@bastelfreak bastelfreak merged commit 13df6a8 into voxpupuli:master Mar 9, 2019
@bastelfreak bastelfreak deleted the types branch March 9, 2019 10:36
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