Update modules with defined types for variables as described in docs/Add reference.md #440

danquack · 2018-10-21T02:37:16Z

I couldn't find where you ran your merge script for updating the changelog, but I imagine an additional command of puppet strings generate can be run to make this automated and up to date.

Pull Request (PR) description

Based on the current docs, defined types for variables. Then executed rake strings to generate the reference markdown file for forge.

This Pull Request (PR) fixes the following issues

Fixes #439
Fixes #441

bastelfreak · 2018-10-21T06:38:35Z

Hi @danquack. We had some longer discussions in our IRC channel and at voxpupuli/modulesync_config#397. For now we want a REFERENCE.md with puppet-strings docs and maybe not the html docs. You can generate the markdown file with:

bundle exec rake strings:generate\[',,,,false,true']

The output would need some adjustments. Some comments in the pp files are in the wrong format. I you want you can contribute that.

ekohl · 2018-10-21T14:41:39Z

Gemfile

@@ -66,6 +66,7 @@ group :release do
  gem 'puppet-blacksmith',           :require => false
  gem 'voxpupuli-release',           :require => false, :git => 'https://github.com/voxpupuli/voxpupuli-release-gem'
  gem 'puppet-strings', '>= 1.0',    :require => false
+  gem 'yard',                        :require => false


puppet-strings requires this so it's redundant

ekohl

A good time to review the class parameters / docs.

REFERENCE.md

ekohl · 2018-10-22T16:37:53Z

REFERENCE.md

+
+##### `dev`
+
+Data type: `Any`


Probably out of scope, but perhaps this should also be an Enum['absent', 'present', 'latest']

ekohl · 2018-10-22T16:38:22Z

REFERENCE.md

+
+Data type: `Enum['absent', 'present', 'latest']`
+
+Desired installation state for python-virtualenv. Boolean values are deprecated


With the data type this looks outdated. Could you remove the mention of boolean values and the allowed values because Enum already tells that.

ekohl · 2018-10-22T16:38:50Z

REFERENCE.md

+
+Data type: `Enum['absent', 'present', 'latest']`
+
+Desired installation state for Gunicorn. Boolean values are deprecated.


Same as above

ekohl · 2018-10-22T16:39:09Z

REFERENCE.md

+Data type: `Optional[Enum['pip', 'scl', 'rhscl', 'anaconda', '']]`
+
+What provider to use for installation of the packages, except gunicorn and Python itself.
+Allowed values: 'pip'


Can also be removed

ekohl · 2018-10-22T16:40:36Z

REFERENCE.md

+
+Data type: `Any`
+
+Set if the gunicorn config directory should be created. Default: false


The default is already given so can be removed here

ekohl · 2018-10-22T16:41:00Z

REFERENCE.md

+Data type: `Any`
+
+Set the application module name for gunicorn to load when not using Django.
+Default: app:app


Redundant default

ekohl · 2018-10-22T16:41:09Z

REFERENCE.md

+
+Allows setting environment variables for the gunicorn service. Accepts a
+hash of 'key': 'value' pairs.
+Default: false


Redundant default

ekohl · 2018-10-22T16:41:16Z

REFERENCE.md

+
+Allows setting the gunicorn idle worker process time before being killed.
+The unit of time is seconds.
+Default: 30


Redundant default

ekohl · 2018-10-22T16:42:13Z

REFERENCE.md

+
+Data type: `Any`
+
+Specifies the PATH variable. Default: [ '/bin', '/usr/bin', '/usr/sbin' ]


Outdated and redundant default

danquack · 2018-10-22T21:39:26Z

All good catches @ekohl. Updated.

ekohl

So many data types could be used, but let's consider that out of scope :)

ekohl · 2018-10-22T21:57:20Z

REFERENCE.md

+
+Data type: `Enum['pip', 'pip3']`
+
+version of pip you wish to use. Default: pip


Default can be removed

REFERENCE.md

ekohl · 2018-10-22T21:58:47Z

REFERENCE.md

+Data type: `Enum['absent', 'present', 'latest']`
+
+Desired installation state for the Python package.
+Allowed values: absent, present and latest


ekohl · 2018-10-22T21:59:12Z

REFERENCE.md

+
+##### `ensure`
+
+Data type: `Any`


The description makes it sound like it should be an enum.

ekohl · 2018-10-22T22:01:15Z

REFERENCE.md

+
+Data type: `Any`
+
+Copy system site-packages into virtualenv. Default: don't If virtualenv version < 1.7 this flag has no effect since


Looks like something is missing at the end of the sentence.

ekohl · 2018-10-22T22:02:09Z

REFERENCE.md

+
+Data type: `Any`
+
+reate $venv_dir


danquack · 2018-10-22T22:55:38Z

Better?

…removed remaining defaults

ekohl · 2018-10-23T13:43:29Z

Should we remove docs from README.md and only refer to REFERENCE.md?

danquack · 2018-10-23T16:09:32Z

They were essentially duplicate. And since people look at code, might as well move it and KEEP it in code.

ekohl · 2018-10-24T11:46:17Z

manifests/gunicorn.pp

-  $log_level                     = 'error',
-  $template                     = 'python/gunicorn.erb',
-  $args                         = [],
+  Enum['present', 'abesent'] $ensure  = present,


typo in absent

ekohl · 2018-10-24T11:46:52Z

manifests/gunicorn.pp

+  $access_log_format                  = false,
+  $accesslog                          = false,
+  $errorlog                           = false,
+  $log_level                           = 'error',


nit: odd alignment here

README.md

bastelfreak · 2018-10-24T18:56:48Z

manifests/dotfile.pp

-  $config   = {},
+  Enum['absent', 'present'] $ensure   = 'present',
+  $filename                           = $title,
+  String $owner                       = 'root',


Can you use String[1] here, to prohibit empty strings?

bastelfreak · 2018-10-24T18:57:18Z

manifests/dotfile.pp

+  $filename                           = $title,
+  String $owner                       = 'root',
+  String $group                       = 'root',
+  String $mode                        = '0644',


Can you use the filemode type? https://github.com/puppetlabs/puppetlabs-stdlib/blob/master/types/filemode.pp#L2

bastelfreak · 2018-10-24T18:57:31Z

manifests/dotfile.pp

+  String $owner                       = 'root',
+  String $group                       = 'root',
+  String $mode                        = '0644',
+  $config                             = {},


This can be enforced as a Hash?

bastelfreak · 2018-10-24T18:59:08Z

I'm fine with this as is, thanks for the awesome contribution @danquack ! @ekohl should we merge this as is and improve datatypes in further PRs?

ekohl · 2018-10-25T16:30:39Z

@ekohl should we merge this as is and improve datatypes in further PRs?

Yes, I also saw room for a lot of improvement but I'm fine with merging this now. Actually I'd prefer to do it now because some of those changes might be considered breaking. Probably easier to review if we split it off.

danquack changed the title ~~added puppet string docs~~ added yard docs Oct 21, 2018

ekohl requested changes Oct 21, 2018

View reviewed changes

danquack force-pushed the dev branch from a94560d to 494e027 Compare October 22, 2018 03:55

pip type updates - defined data types, updated for forge reference

8bb994c

danquack force-pushed the dev branch 2 times, most recently from 8528862 to 8dd4a72 Compare October 22, 2018 04:33

Update forge reference.md documentation for remaning manifest files

84f46b5

danquack force-pushed the dev branch from 8dd4a72 to 84f46b5 Compare October 22, 2018 04:36

Added string type to defined type pip, ensure

0faf83a

ekohl reviewed Oct 22, 2018

View reviewed changes

Removed default comments, install/param -> private, typed mode/dev vars

22e3b3b

danquack changed the title ~~added yard docs~~ Update modules with defined types for variables as described in docs/Add reference.md Oct 22, 2018

ekohl reviewed Oct 22, 2018

View reviewed changes

danquack force-pushed the dev branch from f79e27c to 8d69455 Compare October 22, 2018 22:54

danquack force-pushed the dev branch from 8d69455 to 9ed57db Compare October 22, 2018 23:01

fixed typos in comments added some more types, config.pp -> private, …

e63659e

…removed remaining defaults

danquack force-pushed the dev branch from 9ed57db to e63659e Compare October 22, 2018 23:05

moved examples, variable info from READme to REFERENCE.

d8cd414

ekohl reviewed Oct 24, 2018

View reviewed changes

danquack added 2 commits October 24, 2018 08:58

fixed typo/indentation in gunicorn.pp, update readme for module install

8bcfb17

readme update to point release notes to changelog

0e5ea07

bastelfreak reviewed Oct 24, 2018

View reviewed changes

README.md Show resolved Hide resolved

bastelfreak reviewed Oct 24, 2018

View reviewed changes

danquack force-pushed the dev branch from 4067561 to b9b4e34 Compare October 24, 2018 22:44

String to String[1]

a990c0b

danquack force-pushed the dev branch from 57b2b2f to a990c0b Compare October 24, 2018 22:53

ekohl approved these changes Oct 25, 2018

View reviewed changes

wyardley merged commit 81e3715 into voxpupuli:master Oct 26, 2018

wyardley mentioned this pull request Oct 26, 2018

Replace deprecated validate_* functions #443

Merged

danquack deleted the dev branch October 26, 2018 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update modules with defined types for variables as described in docs/Add reference.md #440

Update modules with defined types for variables as described in docs/Add reference.md #440

danquack commented Oct 21, 2018 •

edited

Loading

bastelfreak commented Oct 21, 2018 •

edited

Loading

ekohl Oct 21, 2018

ekohl left a comment

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

danquack commented Oct 22, 2018

ekohl left a comment

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

ekohl Oct 22, 2018

danquack commented Oct 22, 2018

ekohl commented Oct 23, 2018

danquack commented Oct 23, 2018

ekohl Oct 24, 2018

ekohl Oct 24, 2018

bastelfreak Oct 24, 2018

bastelfreak Oct 24, 2018

bastelfreak Oct 24, 2018

bastelfreak commented Oct 24, 2018

ekohl commented Oct 25, 2018


		Data type: `Enum['absent', 'present', 'latest']`

		Desired installation state for python-virtualenv. Boolean values are deprecated


		Data type: `Enum['absent', 'present', 'latest']`

		Desired installation state for Gunicorn. Boolean values are deprecated.


		Data type: `Any`

		Set if the gunicorn config directory should be created. Default: false


		Data type: `Any`

		Specifies the PATH variable. Default: [ '/bin', '/usr/bin', '/usr/sbin' ]


		Data type: `Enum['pip', 'pip3']`

		version of pip you wish to use. Default: pip


		Data type: `Any`

		Copy system site-packages into virtualenv. Default: don't If virtualenv version < 1.7 this flag has no effect since

Update modules with defined types for variables as described in docs/Add reference.md #440

Update modules with defined types for variables as described in docs/Add reference.md #440

Conversation

danquack commented Oct 21, 2018 • edited Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

bastelfreak commented Oct 21, 2018 • edited Loading

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danquack commented Oct 22, 2018

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danquack commented Oct 22, 2018

ekohl commented Oct 23, 2018

danquack commented Oct 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastelfreak commented Oct 24, 2018

ekohl commented Oct 25, 2018

danquack commented Oct 21, 2018 •

edited

Loading

bastelfreak commented Oct 21, 2018 •

edited

Loading