-
-
Notifications
You must be signed in to change notification settings - Fork 881
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
(maint) Better docs #451
Conversation
Going to leave this for just a quick second... let's introduce this after we drop the |
@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? |
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. |
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. |
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 |
@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: 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? |
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... |
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. |
Opened #941 for this |
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