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

(maint) Better docs #451

Closed
wants to merge 2 commits into from
Closed

(maint) Better docs #451

wants to merge 2 commits into from

Conversation

3flex
Copy link
Contributor

@3flex 3flex commented Sep 20, 2014

Moves all docs to the README, using the standard module documentation template recommended by Puppetlabs.

All public classes and defined types now say "refer to README" in the documentation section. All private classes had the documentation comments removed completely.

First step to fixing #401

@jfryman
Copy link
Contributor

jfryman commented Sep 20, 2014

Going to leave this for just a quick second... let's introduce this after we drop the resources namespace. Working on Pulls now...

@mlehner616
Copy link

@3flex This is definitely a huge step towards what I think the end result should look like. It's up to you and @jfryman if you think this PR should be completed fully before merging it especially due to the fact that we're removing the inline comments. Perhaps we can leave the comment docs there until the README gets fully completed?

@3flex
Copy link
Contributor Author

3flex commented Sep 22, 2014

I tried to make sure I pulled everything relevant from the comments, do you see anything missing?

There's some other work going on in #453 so this won't be merged right away since it will soon be out of date, and I'm planning to merge even if not every parameter is documented since I'm hoping by making it so visible others will contribute their own updates. It's too much work to get perfect all on my own before merging. I will document a few parameters fully so they can be used as a model for people to follow when they submit PRs but otherwise it's "done" - at least for now.

If you think something MUST be included then feedback's very welcome. Or you can submit PRs against my branch.

@mlehner616
Copy link

No I agree, I think your approach is a good one to get community involvement on this. It is indeed a lot of work. I seriously appreciate the work myself. As I get into the nginx config more (might be a little while) I will definitely contribute.

@3flex
Copy link
Contributor Author

3flex commented Dec 8, 2014

Hi @jfryman I know we had a very brief email discussion about whether to wait on this while you worked on a method to create some HTML documentation automatically via RDoc that might supersede this work.

I think the Puppet Approved criteria may have shifted since then, as now their documentation requirements all focus on having a README. That can be extended later with Puppet Lab's strings project, which is supposed to pull stuff from both comments and the README and allow generation of HTML documentation automatically which I think would align with what you were trying to achieve.

So I propose that I update the README so it's up to date with master (add any new parameters and update anything that might have changed) and scrap the second commit where I deleted docs from the classes themselves, then merge unless there's anything important missing.

Thoughts? If you want to wait until stuff is moved out of the resource namespace I can work on a PR for that first. Any other blockers? I'll add a note in CONTRIBUTING.md regarding documentation requirements for contributions so things hopefully won't fall behind again.

@jfryman
Copy link
Contributor

jfryman commented Dec 8, 2014

@3flex If you want to take a pass at documents, I don't think we should block to the namespace rename. That's an easy PR once updated.

@3flex 3flex added the enhancement New feature or request label Apr 16, 2015
@wyardley
Copy link
Collaborator

wyardley commented Oct 6, 2016

@3flex: totally support this idea! The fact that there are currently no docs for the nginx and nginx::config is terrible! Not sure what the plans are in terms of using puppet-strings or similar things for self-documentation, but sounds like it's not totally ready.

Is there anything I or "we" can do to help with improving the docs? Is the work in this PR still useful with a rebase, or is it so old that re-using it is going to be hard?

@3flex
Copy link
Contributor Author

3flex commented Oct 9, 2016

I see many conflicts; updating this is probably more trouble that it's worth.

IMHO we should use puppet-strings as the documentation based going forward, making most of this PR redundant.

I think the best way to improve docs is to open a new PR to format the existing inline documentation so it's compatible with puppet strings 0.99.0 which is likely to drop very very soon, and use puppet-strings for documentation going forward (I think this is voxpupuli's direction, see voxpupuli/plumbing#19)

Not sure what the best approach is for adding documentation to nginx and nginx::config classes, other than someone digging in to document it all...

@wyardley
Copy link
Collaborator

wyardley commented Oct 9, 2016

I agree on both counts.

If the version of puppet strings that allows inline docs is really imminent, I'm totally in favor.

I'm going to leave this open for now simply for ease of finding it -- even with all the conflicts, it should still be a good starting point for the descriptions of the various knobs / whistles, at least the ones that existed when this was written.

@wyardley
Copy link
Collaborator

Opened #941 for this

@wyardley wyardley closed this Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants