-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Unit test to require execution module documentation #52368
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I ❤️ better docs!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the doc build failed
Yep. Since the test finds missing docs, I then added lots of missing rst files. Because they've never been used, it turns out there are many typos and errors that Sphinx chokes on. I've then started going through those errors and I'm now up to working through the state errors. So it will probably be another couple of days until I can get everything building again. While needed it's turned out to be a suckier project than first expected. But I now know a bit more about rst syntax than I did before 😋 Relatedly, once it works, we'll want to get this merged quickly as it has required touching lots of module files to fix their documentation. Letting it linger risks constant merge conflicts. |
That makes sense :) Ping me when it's (re)-ready to merge and I'll make sure to get it shepherded 🐑 🚀 😀 |
Down to the last doc error of:
Edit: looks like it's from aws_kms renderer. Taking a look... Any thoughts? Otherwise this looks good to go! Pshew! |
I think we've got a clean build! The unit test only checks that each python file has a corresponding rst file. The doc build process errors if the rst file isn't referenced, forcing the human to add the entry to an index file, but it doesn't clearly state that process. Not sure how often that comes up, but in PRs it should be discovered. Anyway, I think this is all set to go. @waynew care to look it over once more with the team and merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build is now clean
ping @waynew. This should go in before too many conflicting files are updated. |
Hey @mchugh19 It appears that something is wonky with the PR builds. Specifically, they're failing before the tests even start. Could you try rebasing? If that doesn't clear up the tests then we may want to try just opening a new PR, but 🤞 |
Can do, but I'm away on holiday for the next couple of weeks. If anyone
wants to give it a go, please do. Otherwise I'll be back around the start
of May.
…On Wed, Apr 17, 2019, 16:08 Wayne Werner ***@***.***> wrote:
Hey @mchugh19 <https://github.com/mchugh19> It appears that something is
wonky with the PR builds. Specifically, they're failing before the tests
even start. Could you try rebasing? If that doesn't clear up the tests then
we may want to try just opening a new PR, but 🤞
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#52368 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABTB5QEVxagfnLb1xE-SoBQfEKEVIOyJks5vhyrhgaJpZM4cUAYY>
.
|
Pretty sure these other failures are also unrelated - re-running now (they look like they failed in the setup step) |
Can we get this backported to earlier releases? Docs should probably be accurate for supported releases. |
of course! I'll add the backport label and it will get backported when its merged. |
Correct spacing
thanks for this one @mchugh19 ! This will be great to have. I'll try backporting it tomorrow or monday :) |
Unit test to require execution module documentation
Port #52368 doc requirement tests to master
What does this PR do?
Adds unit test to validate that salt module, state, returners, wheel, etc files have a matching rst in docs and checks that doc rst files have matching py.
Also adds missing docs so that the tests pass
Previous Behavior
Modules could be added without documentation link
New Behavior
Unit test will catch that modules do not have associated docs
Tests written?
Yes
Commits signed with GPG?
No