ACI: Move dev_guide from Wiki to Ansible docs

[dagwieers]

SUMMARY

We had this item on our list for some time, as the Wiki is not the right location for documentation.
https://github.com/ansible/community/wiki/Network:-ACI-Development-Guide

ISSUE TYPE

• Docs Pull Request

COMPONENT NAME

aci dev_guide

ANSIBLE VERSION

v2.7

ADDITIONAL INFORMATION

Screenshot of local Table of Contents:

[dagwieers]

cc @jmcgill298

[ansibot]

cc @acozine
click here for bot help

[acozine]

Thanks for contributing content @dagwieers. This PR needs some work. At a minimum, please make Shippable happy, make the headers conform to #35520, add a basic description (what is an ACI module and why would someone want to write their own?), and add a link to this page from the ACI guide. Active headings would make this page better as well. When that's all done, I'll give the text a more thorough read.

[dagwieers]

@mattclay @gundalow @acozine The rstcheck sanity check is being more strict than Sphinx itself. It is complaining about python examples (even snippets of code) whereas Sphinx doesn't care. Which means I now have to ensure that my snippets of code are syntactically correct (which is silly, because we used place-holders to make code more clear or we selected pieces of code instead of full code).

[dagwieers]

@acozine Can you please merge #35520 so a contributor can simply look at the existing documentation to ensure he's using the correct conventions ? We cannot expect contributors to have an active knowledge of open PR's and their authority (as long as something is not merged, IMO it cannot hold authority over what is already merged...)

[dagwieers]

@acozine What are "Active headings" ?

[mattclay]

The syntax checking can be an inconvenience for some examples. Fortunately those cases can be easily resolved with minor syntax changes or a small amount of boilerplate. The alternative would be to disable syntax checking, but that seems like giving up more than we'd be gaining.

[dagwieers]

@mattclay Ah, so this is a known problem. I wonder if there's a way to disable the syntax checking for those examples we know this is not a problem.

We explain the module's structure, by using snippets from a module. Which means we break up if elif else statements into separate examples. There's no easy way to fix that unless we break that structure and then it does no longer reflect the module.

[acozine]

@dagwieers thanks for the quick response - what time is it where you are???

#35520 is an Issue, not a PR, so I can't merge it, but if you look at the recent changes to the Developer Guide (#45179) or the Community Guide (#45364) you will see the header notation in action. I'm working to fix the headers slowly, along with other changes, which is why the issue is still open. If you have specific questions, about header notation, let me know.

[acozine]

By "active headings" I meant giving headings verbs and more detail - for example, "Importing shared ACI code" instead of "Imports". You can ignore that for now - the other changes are more important.

[dagwieers]

@acozine It's 4AM over here. If you need any help fixing the remaining documentation to follow the new rules, let me know. I now based my headings on the Windows dev_guide, because I am not sure what are supposed to be parts and chapters in my document.

PS While improving the docs I was doing exactly that!

[acozine]

Some of the headings in this doc are already active: "Testing for sanity checks" is a good example.

[mattclay]

@dagwieers For the if/elif/else case, perhaps something like the following would suffice:

    if conditional:
        pass  # other logic here
    else
        # actual example here

Yes, this adds boilerplate, but it also passes the tests. Also, for some readers it will be easier to follow since the example is free standing. Otherwise, when starting reading in the middle of the document it can look like part of the example is missing.

[ansibot]

cc @brunocalogero @schunduri
click here for bot help

[mattclay]

I find having the "stub code" at the top of the conditional useful for providing context. But that's just me. I'll leave it up to you and @acozine to work out what you'd like to do.

[dagwieers]

Well, thinking about this even more, I happily give up syntax highlighting if that keeps the examples identical to the modules.

[dagwieers]

@acozine I think I fixed all your remarks. Not all headings are active because I don't think it makes sense everywhere. Feedback appreciated.

[dagwieers]

@acozine Since this is a PR from 2 different authors (the original document and then the changes) I would prefer if both commits are merged using "Rebase and merge" rather than "Squash and merge" so the content is being correctly attributed.

[acozine]

Overall this looks great. Thanks @dagwieers for including anchors for incoming links, as well as for making Shippable happy and fixing the headers. I appreciate your attention to detail. I've found a few little things to fix before merging.

[acozine]

Hyphenate "module-specific parameters" (x2).

[dagwieers]

Done.

[acozine]

Currently present/absent/query are in italics - did you mean them to be in code?

[dagwieers]

The original docs were written by someone else, I guess so.

[acozine]

I'd change the hint to "Do not provide default values for configuration arguments. Default values could cause unintended changes to the object."

[dagwieers]

Done.

[acozine]

Instead of "check_mode", I'd either enclose it in double backticks to make it look like code, or say "check mode" without the underscore. Also, currently state is italic (single backtick), should probably be code (double backticks).

[dagwieers]

Done.

[acozine]

I'd put double backticks around present here.

[dagwieers]

Done.

[acozine]

"is done" is a bit confusing here - maybe "The configuration s built the same way as the object" or "The configuration portion is built just like the object portion"?

[dagwieers]

Done.

[acozine]

Use sentence case for headings (When state is present, not When State is Present).

[dagwieers]

Done.

[acozine]

Sentence case here too.

[dagwieers]

Done.

[acozine]

For anyone else following this PR, the changes are temporarily visible at http://docs.testing.ansible.com/ansible/latest/dev_guide/developing_modules_general_aci.html.

[dagwieers]

@acozine @jmcgill298 Please re-review !

[acozine]

Looks good on the writing side, thanks @dagwieers. I'll let @jmcgill298 weigh in before merging.

[jmcgill298]

LGTM

[dagwieers]

@acozine Can we backport this to v2.7 ?

[acozine]

@dagwieers go ahead and open a backport PR, if we can't get it in before the release, I'll merge it the day after and republish the docs.