MODULES-3774: stdlib validate_legacy review

1 files changed, 37 insertions, 30 deletions

diff --git a/README.markdown b/README.markdown
index 667da33..d27668b 100644
--- a/README.markdown
+++ b/README.markdown
@@ -304,7 +304,7 @@ Deletes all instances of the undef value from an array or hash. For example, `$h
 
 #### `deprecation`
 
-A function to print deprecation warnings, logs a warning once for a given key. The uniqueness key - can appear once. The msg is the message text including any positional information that is formatted by the user/caller of the method. It is affected by the puppet setting 'strict', which can be set to :error (outputs as an error message), :off (no message / error is displayed) and :warning (default, outputs a warning). In Puppet version less than 4.0 an environment variable can be set to decide whether or not to log deprecation warnings (ENV[STDLIB_LOG_DEPRECATION]). If this environment variable is set to true, the functions will log a warning. *Type*: String, String.
+Prints deprecation warnings and logs a warning once for a given key. The uniqueness key - can appear once. The msg is the message text, including any positional information that is formatted by the user or caller of the method. The Puppet setting 'strict' affects this function. Set 'strict' to :error to output an error message, :off to output no message or error, or :warning (default) to output a warning. In Puppet versions lower than 4.0, you can set an environment variable to decide whether or not to log deprecation warnings (`ENV[STDLIB_LOG_DEPRECATION]`). If this environment variable is set to true, the functions log a warning. *Type*: String, String.
 
 #### `difference`
 
@@ -1370,43 +1370,59 @@ The following values will fail, causing compilation to abort:
 
 #### `validate_legacy`
 
-This function supports updating modules to upgrade from Puppet 3 style argument validation (using the stdlib `validate_*` functions) to Puppet 4 data types, without breaking the world.
+Validates a value against both a specified type and a deprecated validation function. Silently passes if both pass, errors if only one validation passes, and fails if both validations return false.
 
-##### For module users
+Accepts arguments for: 
+* the type to check the value against,
+* the full name of the previous validation function,
+* the value to be checked,
+* an unspecified number of arguments needed for the previous validation function.
+
+Example:
 
-You are probably here, because you're receiving loads of deprecation warnings about `validate_*` functions. If you're still on Puppet 3, use the options described in [`deprecation`](#deprecation) to silence the messages for now, and avoid upgrading to module versions which already have dropped Puppet 3 support. If you're already running Puppet 4 (shipped in Puppet Enterprise 2015.2 or later), please read on to understand the required steps to fix those issues.
+```
+validate_legacy("Optional[String]", "validate_re", "Value to be validated", ["."])
+```
 
-The `validate_*` functions were the only way on Puppet 3 to easily check the types of class and define arguments. Additionally some of the functions provided additional helpers like [validate_numeric](#validate_numeric), which would not only allow numbers, but also arrays of numbers. Puppet 4 now allows much better defined type checking using [data types](https://docs.puppet.com/puppet/latest/reference/lang_data.html), without such surprises. To avoid breaking the world for people depending on those surprises, `validate_legacy` provides a way to make those edge-cases visible, and assist people with getting onto the clearer Puppet 4 syntax.
+This function supports updating modules from Puppet 3 style argument validation (using the stdlib `validate_*` functions) to Puppet 4 data types, without breaking functionality for those depending on Puppet 3 style validation.
 
-Depending on the current state of development of the modules you use and the data you feed them, you'll encounter different messages:
+##### For module users
 
-* "Notice: Accepting previously invalid value for target type '<type>'": Nothing to worry, you're already using values allowed by the new type, that would have been invalid by the old validation function. This is informational only.
-* "Warning: This method is deprecated, please use the stdlib validate_legacy function": The module you're using hasn't yet upgraded to validate_legacy, use the options from [deprecation()](#deprecation) to silence the warnings for now, or submit a patch with you module's developer. See below for details.
-* "Warning: validate_legacy(<function>) expected <type> value, got <actual type>": Your code is passing a value to the module that was accepted by the Puppet v3 style validation, but that will not be accepted by the next version of the module. Most often this can be fixed by removing quotes from numbers, or booleans.
-* "Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, validate_legacy(<function>) expected <type> value, got <actual type>": Your code is passing a value that is not acceptable to either the new, or the old style validation.
+If you are running Puppet 4 and receiving deprecation warnings about `validate_*` functions, the `validate_legacy` function can help you find and resolve the deprecated code.
 
+In Puppet 3, the `validate_*` functions were the only way to easily check the types of class and defined type arguments. Some of the functions provided additional helpers like [validate_numeric](#validate_numeric), which unintentionally allowed not only numbers, but also arrays of numbers. Puppet 4 allows much better defined type checking using [data types](https://docs.puppet.com/puppet/latest/reference/lang_data.html), without such unintentional effects. The `validate_legacy` function makes these differences visible and makes it easier to move to the clearer Puppet 4 syntax.
 
+Depending on the current state of development of the modules you use and the data you feed those modules, you'll encounter different messages:
 
+* `Notice: Accepting previously invalid value for target type '<type>'`: This message is informational only. You're using values that are allowed by the new type, but would have been invalid by the old validation function.
+* `Warning: This method is deprecated, please use the stdlib validate_legacy function`: The module has not yet upgraded to `validate_legacy`. Use the [deprecation](#deprecation) options to silence warnings for now, or submit a fix with the module's developer. See the information [for module developers](#for-module-developers) below for how to fix the issue.
+* `Warning: validate_legacy(<function>) expected <type> value, got <actual type>_`: Your code passes a value that was accepted by the Puppet 3 style validation, but will not be accepted by the next version of the module. Most often, you can fix this by removing quotes from numbers or booleans.
+* `Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, validate_legacy(<function>) expected <type> value, got <actual type>`: Your code passes a value that is not acceptable to either the new or the old style validation.
 
 ##### For module developers
 
-Many `validate_*` functions have surprising holes in their validation. For example, [validate_numeric](#validate_numeric) would not only allow numbers, but also arrays of numbers, or strings that look like numbers, without giving you any control over the specifics. Contrast that to Puppet 4 [data types](https://docs.puppet.com/puppet/latest/reference/lang_data.html) which allow you to choose between `Numeric`, `Array[Numeric]`, or `Optional[Numeric]`. To get from here to there, without leaving your users behind, the validate_legacy function will provide you with a tool to make this migration as painless as possible.
+Many `validate_*` functions have surprising holes in their validation. For example, [validate_numeric](#validate_numeric) allows not only numbers, but also arrays of numbers or strings that look like numbers, without giving you any control over the specifics. In contrast, Puppet 4 [data types](https://docs.puppet.com/puppet/latest/reference/lang_data.html) allows you to choose between `Numeric`, `Array[Numeric]`, or `Optional[Numeric]`. The `validate_legacy` function helps you move from Puppet 3 style validation to Puppet 4 validation without breaking functionality your module's users depend on.
 
-To start out, for each parameter of your classes and defines, you'll have to decide on a new Puppet 4 data type to use. In most cases the new data type will allow a different set of values than the original `validate_*` function (which is the point of the whole exercise). The situation then looks like this:
+For each parameter of your classes and defined types, choose a new Puppet 4 data type to use. In most cases, the new data type allows a different set of values than the original `validate_*` function. The situation then looks like this:
 
 |              | `validate_` pass | `validate_` fail |
 | ------------ | ---------------- | ---------------- |
 | matches type | pass             | pass, notice     |
 | fails type   | pass, deprecated | fail             |
 
-The code after the validation will still have to deal with all possible values for now, but users of your code can now change their manifests to only pass values that will match the new type.
+The code after the validation still has to handle all possible values for now, but users of your code can change their manifests to pass only values that match the new type.
+
+For each `validate_*` function in stdlib, there is a matching `Stdlib::Compat::*` type that allows the appropriate set of values. See the documentation in the `types/` directory in the stdlib source code for caveats.
+
+For example, given a class that should accept only numbers, like this:
 
-To effect those checks, given a class like this:
 ~~~
 class example($value) {
   validate_numeric($value)
 ~~~
-which should only accept numbers, the resulting validation code looks like this:
+
+the resulting validation code looks like this:
+
 ~~~
 class example(
   Variant[Stdlib::Compat::Numeric, Numeric] $value
@@ -1414,26 +1430,17 @@ class example(
   validate_legacy(Numeric, 'validate_numeric', $value)
 ~~~
 
-The type of `$value` is defined as `Variant[Stdlib::Compat::Numeric, Numeric]`, which will allow any `Numeric` (the new type), and all values previously accepted by validate_numeric (through `Stdlib::Compat::Numeric`). For each `validate_*` function in stdlib, there is a matching `Stdlib::Compat::*` type that will allow the appropriate set of values through. See the documentation in the `types/` directory in the stdlib source code for caveats.
-
-The call to `validate_legacy` will take care of triggering the correct log or fail message for you. It requires the new type, the previous validation function name, and all arguments to that function.
+Here, the type of `$value` is defined as `Variant[Stdlib::Compat::Numeric, Numeric]`, which allows any `Numeric` (the new type), as well as all values previously accepted by `validate_numeric` (through `Stdlib::Compat::Numeric`). 
 
-If you were previously still supporting Puppet 3, this is a breaking change. You'll need to update your `metadata.json` to not support Puppet 3 anymore in the `requirements` section, and bump the major version of your module. This should still pass all previously existing tests your module has. Do not forget to add more tests for the newly possible values. This is also a good time to start calling [`deprecation()`](#deprecation) for all parameters you always wanted to get rid of, or add additional constraints on your parameters.
+The call to `validate_legacy` takes care of triggering the correct log or fail message for you. It requires the new type, the previous validation function name, and all arguments to that function.
 
-After releasing this version, you can at any time release another breaking change release where you remove all compat types, and all calls to `validate_legacy`. At this time you can also go through your code and remove all leftovers dealing with the previously possible values.
+If your module still supported Puppet 3, this is a breaking change. Update your `metadata.json` requirements section to indicate that your module no longer supports Puppet 3, and bump the major version of your module. With this change, all existing tests for your module should still pass. Create additional tests for the new possible values.
 
-At all times note in the CHANGELOG and the README at what stage of the process your modules currently are.
+As a breaking change, this is also a good time to call [`deprecation`](#deprecation) for any parameters you want to get rid of, or to add additional constraints on your parameters.
 
-##### Reference
+After releasing this version, you can release another breaking change release where you remove all compat types and all calls to `validate_legacy`. At that time, you can also go through your code and remove any leftovers dealing with the previously possible values.
 
-Validates a value against both a specified type and a deprecated validation function. Silently passes if both pass, errors if one validation passes and the other does not, fails if both validations return false.
-Arguments include the type to check the value against, the full name of the previous validation function, the value itself to be checked, and an unspecified amount of arguments needed for the previous validation function.
-
-Example:
-
-  ~~~
-  validate_legacy("Optional[String]", "validate_re", "Value to be validated", ["."])
-  ~~~
+Always note such changes in your CHANGELOG and README.
 
 #### `validate_numeric`