Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use puppet-strings for reference docs #58

Merged
merged 2 commits into from
Jan 14, 2020

Conversation

alexjfisher
Copy link
Member

Not all parameters are currently documented. This commit migrates the
ones that were documented in the README to REFERENCE.md using puppet
strings.

@alexjfisher
Copy link
Member Author

The parameters currently missing documentation are...

  • config_keys_manage
  • package_source
  • package_provider
  • pools
  • clientlog
  • clientloglimit

I decided to leave them out of this PR and will create a separate PR for them. I can also create follow-up PRs to

  • Add some more data types
  • Move non operating specific defaults out of params.pp and into init.pp so that puppet-strings can use them

Let me know what you think. Again, I didn't want to put too much in this single PR (making it hard to review).

@@ -111,6 +111,9 @@ class { '::chrony':
port => 123,
}
```
#### Note
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note had got separated from its example.

# main chrony class
# @summary Installs and configures chrony
#
# @example Install chrony with default options
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've copied the examples from the README, but not deleted them (yet). I think it's nice that there are at least a few of the most important examples there and not just in the reference docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

# stepping the time may be wanted. In this case, set `makestep_updates` to `-1`
# to allow stepping the time for any update.
# @param makestep_updates
# Together with `makestep_seconds`, this configures the `makestep` parameter of `chronyd`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure what to do here. Happy to change this to new wording (to remove the duplication), or perhaps this could be done in a future PR.

Copy link
Contributor

@aboe76 aboe76 Jan 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reword this, it's horrible to get your head around these sentences.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded and linked to official docs (which aren't great either TBH). I've also added an @example and data types to the parameters.

@aboe76
Copy link
Contributor

aboe76 commented Jan 11, 2020

@alexjfisher nice work,

Not all parameters are currently documented.  This commit migrates the
ones that were documented in the README to REFERENCE.md using puppet
strings.
@aboe76
Copy link
Contributor

aboe76 commented Jan 13, 2020

@alexjfisher is it ready for merging?

@alexjfisher
Copy link
Member Author

@alexjfisher is it ready for merging?

yes. I was going to leave further improvements to future PRs. I didn't want to mix too many things in one diff making it hard to review.

@aboe76 aboe76 merged commit 5568d68 into voxpupuli:master Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants