Optimizing puppet var types about puppetlabs-tomcat HOT 4 CLOSED

Hi @Joris29, thank for pointing out these potential improvements. One of the primary reasons why we didn't use very strict types was to avoid a very abrupt backwards-incompatible update. Some changes of the mentioned changes can probably get away with a stricter datatype right away and we will look into it.

The $source_url suggestion, however, cannot be implemented as (afaik) the parameter is used to contain a full URL that starts with either puppet://, http(s):// or ftp://, rather than exclusively those three String values. You can see examples of this happening in our Spec test files. Maybe the wording of this parameter description could be adjusted to be a little less vague.

I cannot ensure either that the $address change will work as we seem to permit the value 'localhost' as a valid option, while our custom type Stdlib::IP::Address doesn't specify that as a true value.

We will investigate these and more possible parameter type updates soon. However, if there is any potential upgrade/bugfix that you need immediately merged into the module, feel free to drop a Pull Request and we will review/merge it as soon as we can.

from puppetlabs-tomcat.

@Joris29 A limitation regarding our custom Port type is that it allows exclusively Integers. Which might break the builds of users that were passing Strings. That one might be have to be remade as a Variant[String[1], Stdlib::Port] to avoid breaking changes.

The $address suggestion is a fairly good idea. Can't believe I didnt think about it before 😅

from puppetlabs-tomcat.

The one that bothers me most at the moment is the port being a string it just feels weird but I'll see if i can make a PR

from puppetlabs-tomcat.

And about the address you could work with a variant either being Stdlib::IP::Address or localhost
Something like this:
Variant[Stdlib::IP::Address, 'localhost']

from puppetlabs-tomcat.