PaloAltoNetworks / terraform-azurerm-vmseries-modules

Terraform Reusable Modules for VM-Series on Azure
https://registry.terraform.io/modules/PaloAltoNetworks/vmseries-modules/azurerm/latest
MIT License
49 stars 58 forks source link

unify the format of variables documentation #257

Closed FoSix closed 12 months ago

FoSix commented 1 year ago

this is related mainly to examples, but module might require review as well proposed format - VMSS documentation in autoscaling example

FoSix commented 1 year ago

Proposal of a description block for variables used in examples. This covers variables that are complex maps. For simple variables (string, number, bool, etc) standard variable block should be used.

description =  <<-EOF
Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. 

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
  - `name_of_property`: (`variable_type` | `required`) some short description.
  - `example_string_required`: (`string` | `required`) for required properties
  - `example_bool_optional`: (`bool` | `false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional`: (`string` | `module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional`: (`number` | `cloud default`) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
EOF

this would format to a table row looking more or less like this:

Name Description Type Default Required
example_docs Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
- name_of_property: (variable_type | required) some short description.
- example_string_required: (string | required) for required properties
- example_bool_optional: (bool | false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional: (string | module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional: (number | cloud default) when property is not required and we rely on the defaults set by the cloud API


Example

example blocks could follow here
any n/a yes
michalbil commented 1 year ago

First proposal from my side - use hyphen instead of colons to separate property name and description (such an approach is a standard for example in TF resources docs) and remove extra spaces from (type|required) - pipe alone is a good enough separator imo.

description = <<-EOF
Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. 

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
  - `name_of_property` - (`variable_type`|`required`) some short description.
  - `example_string_required` - (`string`|`required`) for required properties
  - `example_bool_optional` - (`bool`|`false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional` - (`string`|`module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional` - (`number`|`cloud default`) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
EOF
Name Description Type Default Required
example_docs Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
- name_of_property - (variable_type|required) some short description.
- example_string_required - (string|required) for required properties
- example_bool_optional - (bool|false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional - (string|module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional - (number|cloud default) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
any n/a yes

Second proposal is similar, with additional alignment in the description - this ha no impact on the generated table row, but is easier to read in the variables.tf directly.

description = <<-EOF
Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. 

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
  - `name_of_property`        - (`variable_type`|`required`) some short description.
  - `example_string_required` - (`string`|`required`) for required properties
  - `example_bool_optional`   - (`bool`|`false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional` - (`string`|`module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional` - (`number`|`cloud default`) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
EOF
Name Description Type Default Required
example_docs Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
- name_of_property - (variable_type|required) some short description.
- example_string_required - (string|required) for required properties
- example_bool_optional - (bool|false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional - (string|module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional - (number|cloud default) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
any n/a yes

Third proposal removes additional code markers within the braces. The most important part - property name - is emphasized, its description, including type etc. is in the same line, still easy to find and read.

description = <<-EOF
Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. 

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
  - `name_of_property`        - (variable_type|required) some short description.
  - `example_string_required` - (string|required) for required properties
  - `example_bool_optional`   - (bool|false) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional` - (string|module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional` - (number|cloud default) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
EOF
Name Description Type Default Required
example_docs Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

If this is a map, information about keys and values meaning should be included.

If values are nested maps, a list of keys/properties could follow:
- name_of_property - (variable_type|required) some short description.
- example_string_required - (string|required) for required properties
- example_bool_optional - (bool|false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional - (string|module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional - (number|cloud default) when property is not required and we rely on the defaults set by the cloud API

Example

example blocks could follow here
any n/a yes
FoSix commented 1 year ago

@michalbil :

FoSix commented 1 year ago

Another idea would be to split the important and not important properties into to two lists. This can be optional, but would become handy for large modules, like vmss or appgw. Sometimes you're not interested in all options available, or the defaults suit VMSERIES quite good, so it's a waste of time on reading when you mix them with the required/important ones.

One thing more is that the list of required/important props is usually shorter, so the top of the documentation would be less cluttered.

Another option is to add some HTML tags like <detailed> to allow some folding when displaying the documentation on GH/TF Registry.

So the description would look like this:

description = <<-EOF
Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliq.....

If this is a map, information about keys and values meaning should be included.

Required/important/without defaults, etc:
  - `name_of_property`: (`variable_type` | `required`) some short description.
  - `example_string_required`: (`string` | `required`) for required properties
  - `example_bool_optional`: (`bool` | `false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional`: (`string` | `module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional`: (`number` | `cloud default`) when property is not required and we rely on the defaults set by the cloud API

<details>
<summary>Optional/not required/with defaults, etc:</summary>
  - `name_of_property`: (`variable_type` | `required`) some short description.
  - `example_string_required`: (`string` | `required`) for required properties
  - `example_bool_optional`: (`bool` | `false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional`: (`string` | `module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional`: (`number` | `cloud default`) when property is not required and we rely on the defaults set by the cloud API
  - `name_of_property`: (`variable_type` | `required`) some short description.
  - `example_string_required`: (`string` | `required`) for required properties
  - `example_bool_optional`: (`bool` | `false`) when property is not required and we force a default value in the example, possibly overwrting the module default value
  - `example_string_optional`: (`string` | `module default`) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has `nullable` set to `false`
  - `example_number_optional`: (`number` | `cloud default`) when property is not required and we rely on the defaults set by the cloud API
</details>

<details>
<summary>Examples</summary>
  example blocks could follow here
</details>

EOF
This would render into: Name Description Type Default Required
test Some general variable description.

Some longer description if required: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliq.....

If this is a map, information about keys and values meaning should be included.

Required/important/without defaults, etc:
- name_of_property: (variable_type | required) some short description.
- example_string_required: (string | required) for required properties
- example_bool_optional: (bool | false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional: (string | module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional: (number | cloud default) when property is not required and we rely on the defaults set by the cloud API


Optional/not required/with defaults, etc:
- name_of_property: (variable_type | required) some short description.
- example_string_required: (string | required) for required properties
- example_bool_optional: (bool | false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional: (string | module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional: (number | cloud default) when property is not required and we rely on the defaults set by the cloud API
- name_of_property: (variable_type | required) some short description.
- example_string_required: (string | required) for required properties
- example_bool_optional: (bool | false) when property is not required and we force a default value in the example, possibly overwrting the module default value
- example_string_optional: (string | module default) when property is not required and we rely on module's defaults, this implicitly states that the variable defined in a module has nullable set to false
- example_number_optional: (number | cloud default) when property is not required and we rely on the defaults set by the cloud API



Examples
example blocks could follow here
map {} no
FoSix commented 1 year ago

an example, a sum of all above suggestions can be found:

FoSix commented 12 months ago

all ideas gathered there will be used in #307