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.

Sign up for GitHub

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

Jump to bottom

Update REFERENCE.md, remove docs dir from master #744

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Update REFERENCE.md, remove docs dir from master #744

merged 1 commit into from
Dec 7, 2018

Conversation

wyardley
Copy link
Contributor

@wyardley wyardley commented Dec 6, 2018

Pull Request (PR) description

Update REFERENCE.md, as docs haven't been regenerated for a while.
We should either not push the HTML docs, or put them in gh-pages. This is still
under discussion (voxpupuli/modulesync_config#397).

I can also update gh_pages branch via the rake task if you'd like.

@wyardley wyardley requested review from bastelfreak and ekohl December 6, 2018 08:22
@wyardley wyardley added the docs Improvements or additions to documentation label Dec 6, 2018
ekohl
ekohl requested changes Dec 6, 2018
View reviewed changes
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.

👍 for removing the generated HTML

REFERENCE.md Outdated Show resolved Hide resolved
REFERENCE.md Outdated Show resolved Hide resolved
@wyardley wyardley force-pushed the wyardley-update-remove-generated-docs branch 4 times, most recently from 51404d0 to 150b974 Compare December 6, 2018 15:48
@wyardley
Copy link
Contributor Author

wyardley commented Dec 6, 2018

@ekohl @bastelfreak Fixed some other issues / typos with the source docs.

I can switch it to be a single commit with the changes to source and then the generated docs in another if that's better.

ekohl
ekohl reviewed Dec 6, 2018
View reviewed changes
REFERENCE.md Outdated
@@ -35,7 +35,7 @@

rabbitmq

 @param $loopback_users. default defined in param.pp. This option configures a list of users to allow access via the loopback interfaces
 @param loopback_users This option configures a list of users to allow access via the loopback interfaces
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it still doesn't recognize it as a parameter. Not sure why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that. Not sure why, but still get the error

[warn]: Missing @param tag for parameter 'loopback_users' near manifests/init.pp:192.

Copy link
Member

Choose a reason for hiding this comment

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

You could try adding a line with just a # after is and before the class definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that, didn't seem to make a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An empty line with no hash seems to work... I think that's right? The diffs are hard to review now.

REFERENCE.md Outdated Show resolved Hide resolved
@wyardley wyardley force-pushed the wyardley-update-remove-generated-docs branch 2 times, most recently from c3b8cd2 to a0f11dc Compare December 6, 2018 16:07
wyardley
wyardley commented Dec 6, 2018
View reviewed changes
REFERENCE.md Outdated
@@ -5,7 +5,7 @@

**Classes**

* [`rabbitmq`](#rabbitmq): A module to manage RabbitMQ
* [`rabbitmq`](#rabbitmq):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ekohl not sure why the @summary disappears now? So maybe that leading # rabbitmq is necessary?

@wyardley
Copy link
Contributor Author

wyardley commented Dec 6, 2018

The examples are all messed up too, will have to spend some more time figuring out what it’s barfing on

@ekohl
Copy link
Member

ekohl commented Dec 6, 2018

See my changes in ekohl@57b0b17

I've replaced the spaces and then it does pick it up. The reason is that there's a weird unicode character after the # on that line. Using cat -vet will show this to you:

$ cat -vet manifests/init.pp | grep loopback_users | head -n 1
#M-BM- @param $loopback_users. default defined in param.pp. This option configures a list of users to allow access via the loopback interfaces$

@voxpupuli voxpupuli deleted a comment from ekohl Dec 6, 2018
@wyardley
Copy link
Contributor Author

wyardley commented Dec 6, 2018

Thanks. Do you want me to try and cherry pick that off, or do you want to submit it?

@wyardley wyardley force-pushed the wyardley-update-remove-generated-docs branch from a0f11dc to 64ef64e Compare December 6, 2018 18:17
@wyardley
Copy link
Contributor Author

wyardley commented Dec 6, 2018

$ cat -vet manifests/init.pp
On OS X, this renders normally for me too. od -c shows:

0026020    a   d   m   i   n       f   i   l   e  \n   #      **   @   p
0026040    a   r   a   m       l   o   o   p   b   a   c   k   _   u   s
0026060    e   r   s       T   h   i   s       o   p   t   i   o   n

Pushed up a version without the formatting changes and without some of the other changes, but happy to close this in favor of your version if you want.

@ekohl
Copy link
Member

ekohl commented Dec 6, 2018

Thanks. Do you want me to try and cherry pick that off, or do you want to submit it?

Feel free to pick it into your branch and improve from there.

@wyardley
Update REFERENCE.md, remove docs dir from master
4d8c3dc 
Special thanks to ekohl for additional fixes and formatting.

* Fix a misplaced unicode character causing some problems with docs generation.
* Update REFERENCE.md, as docs haven't been regenerated for a while.  We should
either not push the HTML docs, or put them in gh-pages. This is still under
discussion.
@wyardley wyardley force-pushed the wyardley-update-remove-generated-docs branch from 64ef64e to 4d8c3dc Compare December 6, 2018 18:52
@wyardley
Copy link
Contributor Author

wyardley commented Dec 6, 2018

Also created https://tickets.puppetlabs.com/browse/PDOC-271

ekohl
ekohl approved these changes Dec 6, 2018
View reviewed changes
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.

You may want to take a look at other classes (management and server) to see how they render, but this is already a big improvement for the module

REFERENCE.md
* `rabbitmq::install`: Ensures that rabbitmq-server exists
* `rabbitmq::install::rabbitmqadmin`: Install rabbitmq admin
* `rabbitmq::params`: OS Specific parameters and other settings
* `rabbitmq::repo::apt`: requires puppetlabs-apt puppetlabs-stdlib
Copy link
Member

Choose a reason for hiding this comment

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

Probably could use a better summary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird spacing too.
Yes, there are a lot of gaps, and some undocumented params.
I'm trying not to let the perfect be the enemy of the good, but would welcome any help improving the docs.

Also, if we can figure out the correct process for generated docs, and have them auto-generated as part of the release process, that would be ideal.

@wyardley wyardley merged commit 2378af3 into voxpupuli:master Dec 7, 2018
@wyardley wyardley deleted the wyardley-update-remove-generated-docs branch December 7, 2018 00:25
@wyardley wyardley mentioned this pull request Dec 7, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

@bastelfreak bastelfreak Awaiting requested review from bastelfreak

Assignees
No one assigned
Labels
docs Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wyardley @ekohl