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

Ensure all parameters are documented #151

Merged
merged 6 commits into from May 3, 2020
Merged

Ensure all parameters are documented #151

merged 6 commits into from May 3, 2020

Conversation

smortex
Copy link
Member

@smortex smortex commented Apr 24, 2020

The module has a lot of puppet strings but some parameters lack documentation. This PR intends to add the missing documentation along with tooling to alert contributors when such documentation is missing.

Current status:

  • Match actual parameters order for parameters documentation
  • Align documentation vertically for better legibility
  • Add a unit test that ensures puppet-strings is happy
  • Remove dashed between parameter name and documentation (output to the documentation otherwise)
  • Add the missing documentation

@jflorian
Copy link
Contributor

Just dropping in real quick to offer an immense THANK YOU for this. Not that things were that bad, but this is pure goodness that's usually not the most fun thing to do.

manifests/storage.pp Outdated Show resolved Hide resolved
@smortex
Copy link
Member Author

smortex commented Apr 24, 2020

Just added REFERENCE.md to the repo… Live preview here:
https://github.com/xaque208/puppet-bacula/blob/doc/REFERENCE.md

@zachfi
Copy link
Contributor

zachfi commented Apr 25, 2020

This is looking great, thank you @smortex.

@smortex smortex changed the title WIP: Ensure all parameters are documented Ensure all parameters are documented Apr 25, 2020
@smortex
Copy link
Member Author

smortex commented Apr 25, 2020

Cool ! This seems fair so far :-) @xaque208 i have not dig in the Travis-CI failure, the failing test having some "DEPLOY_TO_FORGE=yes" makes me feel good to see it failing… Is this something that is completely fine and safe and that we can fix, or is something missing, e.g. something that ensure this is running only when a new tag is pushed?

@zachfi
Copy link
Contributor

zachfi commented Apr 25, 2020

Oh I always forget to disable that bit when I rebase the modulesync repo, but yes disabling that entirely has been my aim. Though, I could probably be convinced otherwise considering how bad I've been about cutting releases.

@smortex
Copy link
Member Author

smortex commented Apr 25, 2020

I never released anything to the forge (yet… it's still something we intend to do for some or the opus-codium modules), so I can't really give advice here unfortunately, but tell me if there is anything I can do that would help.

@zachfi
Copy link
Contributor

zachfi commented May 2, 2020

I synced some testing changes to master that might help the tests along here.

@smortex
Copy link
Member Author

smortex commented May 3, 2020

Nice! I rebased my branch on top of master and it seems to be good :-)

While here, also normalise documentation strings: no coma at end of
description.
While here, add tooling to ensure that the REFERENCE.md file is
up-to-date.
@smortex
Copy link
Member Author

smortex commented May 3, 2020

So, the "unused" parameter was used :-) I rebased the commits on top of master and ameded them to fix this "unused" parameter.

@zachfi
Copy link
Contributor

zachfi commented May 3, 2020

Nice. This is great, thank you for your efforts here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants