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

add typedef and class documentation #148

Merged
merged 7 commits into from
Apr 28, 2020

Conversation

TillHein
Copy link

Pull Request (PR) description

This Pull Requests adds in-file documentation for typedefs and classes, to meet the puppet Style Guide criteria (Documentation on line 1 ).
The typedef documentation was taken from the Readme, with the removal of some of the markdown notation to improve readability.

This Pull Request (PR) fixes the following issues

puppet-lint failure (http://puppet-lint.com/checks/documentation/)

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I like this a lot, but can you use puppet-strings documentation? This allows us to generate a nice REFERENCE.md in an automated way. If you need help with this, we'll gladly help.

manifests/acl.pp Outdated Show resolved Hide resolved
manifests/icp_access.pp Outdated Show resolved Hide resolved
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

At this point there's still room for improvement, but I'd be happy with merging it since it's already an improvement over the current situation.

# @param sslproxy_cert_error
# Defaults to undef. If you pass in a hash of sslproxy_cert_error entries, they will be defined automatically. http://www.squid-cache.org/Doc/config/sslproxy_cert_error/
# @param extra_config_sections
# Defaults to empty hash. If you pass in a hash of `extra_config_section` resources, they will be defined automatically.
Copy link
Member

Choose a reason for hiding this comment

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

The defaults to is redundant here. This is what puppet-strings generates:

##### `extra_config_sections`

Data type: `Optional[Hash]`

Defaults to empty hash. If you pass in a hash of `extra_config_section` resources, they will be defined automatically.

Default value: {}

# @param ssl_bump
# Defaults to undef. If you pass in a hash of ssl_bump entries, they will be defined automatically. http://www.squid-cache.org/Doc/config/ssl_bump/
# @param sslproxy_cert_error
# Defaults to undef. If you pass in a hash of sslproxy_cert_error entries, they will be defined automatically. http://www.squid-cache.org/Doc/config/sslproxy_cert_error/
Copy link
Member

Choose a reason for hiding this comment

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

Here the default is somewhat useful:

##### `sslproxy_cert_error`

Data type: `Optional[Hash]`

Defaults to undef. If you pass in a hash of sslproxy_cert_error entries, they will be defined automatically. http://www.squid-cache.org/Doc/config/sslproxy_cert_error/

However, we can also move all the static values from params.pp to init.pp and remove it. That means it's clearer in the docs and less to maintain. Static copies of defaults can go out of sync.

Comment on lines +20 to +24
# would result in the following squid refresh patterns:
# # refresh_pattern fragment for ^ftp
# refresh_pattern ^ftp: 1440 20% 10080
# # refresh_pattern fragment for (/cgi-bin/|\?)
# refresh_pattern (/cgi-bin/|\?) -i 0 0% 0
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't render that well I think since examples are assumes to be puppet code.

# refresh_pattern (/cgi-bin/|\?) -i 0 0% 0
#
#
# @example YAML example
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if you can force puppet-strings to use non-puppet examples.

@TillHein
Copy link
Author

Thanks for the reviews, I mostly agree on the improvement suggestions. But I am not sure what my next point of action is. I'd be happy if this gets merged and improved upon later on.

@ekohl ekohl merged commit 4638374 into voxpupuli:master Apr 28, 2020
@ekohl
Copy link
Member

ekohl commented Apr 28, 2020

Thanks for your efforts. As I said: this is already a huge improvement over the previous state.

@TillHein TillHein deleted the typedef-documentation branch April 29, 2020 07:37
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