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

create puppet strings based reference docs #209

Conversation

your1p
Copy link
Contributor

@your1p your1p commented Apr 17, 2020

Pull Request (PR) description

Create puppet strings based reference docs.

This Pull Request (PR) fixes the following issues

Fixes #158

@your1p your1p changed the title create puppet strings based reference docs WIP:create puppet strings based reference docs Apr 17, 2020
@your1p your1p changed the title WIP:create puppet strings based reference docs create puppet strings based reference docs Apr 22, 2020
Copy link
Member

@Dan33l Dan33l left a comment

Choose a reason for hiding this comment

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

Hi @your1p some comments inline added. Can you update your PR?

@@ -1,5 +1,53 @@
# == Class keepalived
#
# @param sysconf_dir
Copy link
Member

Choose a reason for hiding this comment

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

Since init.pp is potentially the first contact with module by a reader, can you add informations about parameters ?

@@ -1,25 +1,21 @@
# == Define: keepalived::lvs::real_server
# Define Keepalived::Lvs::Real_server
Copy link
Member

Choose a reason for hiding this comment

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

It looks clearer to use the sentence of the summary rather that this.

@@ -11,107 +11,115 @@
# Refer to keepalived's documentation to understand the behaviour
Copy link
Member

Choose a reason for hiding this comment

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

A summary is missing.

# Virtual Server firewall mark. (overrides ip_address and port)
# Default: not set
#
# [*lb_algo*]
# @param lb_algo
# Must be one of rr, wrr, lc, wlc, lblc, sh, dh
# Default: not set.
Copy link
Member

Choose a reason for hiding this comment

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

Since we are using puppet-string the default values are auto-generated. Giving this information by a manual description can be not what is in the code. So please remove them.

@@ -1,167 +1,198 @@
# == Define: keepalived::vrrp::instance
# @summary keepalived::vrrp::instance
Copy link
Member

Choose a reason for hiding this comment

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

It is possible to describe the type by a sentence like a description ?

@@ -1,32 +1,28 @@
# == Define: keepalived::vrrp::script
# @summary keepalived::vrrp::script
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use a sentence more descriptive ?

@@ -1,29 +1,27 @@
# == Define: keepalived::vrrp::sync_group
# @summary keepalived::vrrp::sync_group
Copy link
Member

Choose a reason for hiding this comment

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

Same question here about summary.

@@ -1,24 +1,20 @@
# == Define: keepalived::vrrp::track_process
# @summary keepalived::vrrp::track_process
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@@ -1 +1,3 @@
# @summary keepalived::options
Copy link
Member

Choose a reason for hiding this comment

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

The type is not this but Keepalived::Options but explaining what is describing the data type is better.

@@ -1,3 +1,5 @@
# @summary keepalived::vrrp::instance::vrule
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here.

@vox-pupuli-tasks
Copy link

Dear @your1p, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

1 similar comment
@vox-pupuli-tasks
Copy link

Dear @your1p, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@bastelfreak bastelfreak force-pushed the create-puppet-strings-based-reference-docs branch from 2ae8404 to 3b62dd2 Compare October 2, 2021 10:51
@bastelfreak bastelfreak added enhancement New feature or request and removed merge-conflicts labels Oct 2, 2021
@bastelfreak
Copy link
Member

I did a rebase and will merge this as is. Whoever likes it can add improvements later on.

@bastelfreak bastelfreak merged commit b81c452 into voxpupuli:master Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create puppet-strings based reference docs
3 participants