(CONT-361) Syntax update

[LukasAud]

Code is now compliant with rules regarding:

parameter datatypes
parameter documentation

[smortex]

This looks great!

I tested these changes with onceover with our control-repo configuration to check for regressions and found a backward incompatible changes with the new validation.

The puppetlabs-puppetdb module setup a postgresql::server::db with grant => 'all' (sic):

https://github.com/puppetlabs/puppetlabs-puppetdb/blob/437e1c60c62113865c1b8f340fcbf325fa147a6f/manifests/database/postgresql.pp#L74-L78

The puppetlabs-postgresql module copy the value of the grant parameter to the privilege parameter of a postgresql::server::database_grant defined type:

puppetlabs-postgresql/manifests/server/db.pp

Lines 47 to 51 in 5d62783

	postgresql::server::database_grant { "GRANT ${user} - ${grant} - ${dbname}":
	privilege => $grant,
	db => $dbname,
	role => $user,
	} -> Postgresql_conn_validator<| db_name == $dbname |>

The new code validates that privilege parameter against the type Enum['ALL', 'CREATE', 'CONNECT', 'TEMPORARY', 'TEMP']. The case does not match (Enum is case sensitive) and an error is reported:

  errors:
    Evaluation Error: Error while evaluating a Resource Statement, Postgresql::Server::Database_grant[GRANT puppetdb - all - puppetdb]: parameter 'privilege' expects a match for Enum['ALL', 'CONNECT', 'CREATE', 'TEMP', 'TEMPORARY'], got 'all'
      file: modules/postgresql/manifests/server/db.pp
      line: 47
      factsets: Ubuntu-18.4-64
    Evaluation Error: Error while evaluating a Resource Statement, Postgresql::Server::Database_grant[GRANT puppetdb - all - puppetdb]: parameter 'privilege' expects a match for Enum['ALL', 'CONNECT', 'CREATE', 'TEMP', 'TEMPORARY'], got 'all'
      file: modules/postgresql/manifests/server/db.pp
      line: 47
      factsets: Debian-11-64
    Evaluation Error: Error while evaluating a Resource Statement, Postgresql::Server::Database_grant[GRANT puppetdb - all - puppetdb]: parameter 'privilege' expects a match for Enum['ALL', 'CONNECT', 'CREATE', 'TEMP', 'TEMPORARY'], got 'all'
      file: modules/postgresql/manifests/server/db.pp
      line: 47
      factsets: Debian-10-64
    Evaluation Error: Error while evaluating a Resource Statement, Postgresql::Server::Database_grant[GRANT puppetdb - all - puppetdb]: parameter 'privilege' expects a match for Enum['ALL', 'CONNECT', 'CREATE', 'TEMP', 'TEMPORARY'], got 'all'
      file: modules/postgresql/manifests/server/db.pp
      line: 47
      factsets: CentOS-7.6-64

[SimonHoenscheid]

Great work, this module has been missing data types for a long time! I added a few review comments.

[LukasAud]

New commit. Have adjusted the PR to implement some suggestions.

[ekohl]

I'll note I hate alignment on data types since it creates so much noise in the git log. The old style was much better IMHO.

[LukasAud]

Understandable. However, we are currently favouring a more 'readable' and structured style.

[kenyon]

It doesn't help code readability much, but it makes diffs harder to read and review. +1 for not doing alignment.

[ekohl]

I'd even argue it's less readable if you have a long type (like the version you wrote for the package ensure) there is a lot of whitespace, so it becomes hard to see which data types belong to which variables.

[david22swan]

Personally I find it best for when I have to search out a specific variable in the list, especially for larger modules that have upwards of a dozen.
I've always found it easier to track down that one that I'm looking for.