Skip to content

Commit bb76142

Browse files
committed
Incorporate review feedback on parameter defaults
From the sig-documentation discussion on inline vs Hiera defaults: - Note the readability argument for inline defaults: when defaults live only in Hiera, a reader of init.pp can't tell whether a parameter has a default elsewhere or must be supplied. - Add the required-parameter case to the Optional guidance: when a value is genuinely required with no sensible default, give the parameter no default at all so compilation fails clearly, rather than papering over the requirement with Optional[T] = undef. Signed-off-by: Michael Harp <mike@mikeharp.com>
1 parent 9ec4062 commit bb76142

2 files changed

Lines changed: 6 additions & 2 deletions

File tree

docs/_openvox_8x/bgtm.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,16 @@ Where you put a parameter's default value depends on whether that value is the s
200200
* **Static defaults that are identical across every supported OS** belong inline in the parameter declaration in `init.pp`.
201201
* **OS-specific defaults** belong in module Hiera data with a per-OS hierarchy, resolved through automatic parameter lookup.
202202

203-
Keeping static defaults inline puts the value right where the parameter is declared, which is what module reviewers expect and what keeps the default easy to find.
203+
Keeping static defaults inline puts the value right where the parameter is declared, which is what module reviewers expect and what keeps the default easy to find. When defaults live only in Hiera, someone reading `init.pp` can't tell whether a parameter has a default elsewhere or must be supplied.
204204

205205
Inline defaults also render in generated reference documentation with any toolchain. Defaults placed only in `data/common.yaml` are invisible to upstream [puppet-strings](https://github.com/puppetlabs/puppet-strings/issues/250), though OpenVox's [openvox-strings](https://github.com/voxpupuli/openvox-strings/pull/27) can now read them.
206206

207207
Give each default a single home. Duplicating a value between `init.pp` and module Hiera data means two sources of truth that can drift apart.
208208

209209
Reserve `Optional[T] = undef` for parameters where `undef` is a genuine runtime value, such as a parameter that toggles an optional feature off. Avoid declaring a parameter `Optional` simply to defer its default to Hiera when the parameter will always receive a value, since that misleads users into thinking `undef` is a supported state.
210210

211+
If a parameter is genuinely required and has no sensible default, give it no default at all so that catalog compilation fails clearly when the value is missing, rather than using `Optional[T] = undef`.
212+
211213
For more on the mechanics of parameter defaults and Hiera data in modules, see the [Puppet language style guide](./style_guide.html#parameter-defaults).
212214

213215
### Ordering

docs/_openvox_8x/style_guide.markdown

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ Where a default value lives depends on whether it varies by operating system.
894894

895895
When a parameter's default is the same on every supported operating system, declare it inline in the parameter list.
896896

897-
Keeping static defaults inline puts the value right where the parameter is declared, which keeps it easy to find and is the convention module reviewers expect.
897+
Keeping static defaults inline puts the value right where the parameter is declared, which keeps it easy to find and is the convention module reviewers expect. When defaults live only in Hiera, someone reading `init.pp` can't tell whether a parameter has a default elsewhere or must be supplied.
898898

899899
Inline defaults also render in generated reference documentation with any toolchain. Defaults placed only in `data/common.yaml` are invisible to upstream [puppet-strings](https://github.com/puppetlabs/puppet-strings/issues/250), though OpenVox's [openvox-strings](https://github.com/voxpupuli/openvox-strings/pull/27) can now read them.
900900

@@ -981,6 +981,8 @@ Here `undef` genuinely means the feature is off, so `Optional[T] = undef` is the
981981

982982
Avoid declaring a parameter `Optional` just to defer its default to Hiera when the parameter will always receive a value. Doing so misleads users into thinking `undef` is a supported state when it isn't, and it weakens the type assertion. Declare the real type and give the parameter its actual default instead.
983983

984+
If a parameter is genuinely required and has no sensible default, give it no default at all. A required parameter without a default fails catalog compilation with a clear error when the user omits it, which is safer than papering over the requirement with `Optional[T] = undef`.
985+
984986
### Exported resources
985987

986988
Exported resources should be opt-in rather than opt-out. Your module should not be written to use exported resources to function by default unless it is expressly required.

0 commit comments

Comments
 (0)