Code example improvements in Windows documentation #45055

dagwieers · 2018-09-01T20:52:39Z

SUMMARY

This PR includes:

Using explicit yaml+jinja code-blocks
Capital letter to start a comment or description
Use yes/no instead of true/false

ISSUE TYPE

Docs Pull Request

COMPONENT NAME

Windows docs

ANSIBLE VERSION

v2.7

ansibot · 2018-09-01T20:54:38Z

cc @acozine
click here for bot help

ansibot · 2018-09-01T21:13:25Z

The test ansible-test sanity --test docs-build [explain] failed with 14 errors:

docs/docsite/rst/index.rst:68:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:89:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:148:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:193:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:214:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:216:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:239:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:266:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:275:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:337:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:348:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:370:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:527:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.
docs/docsite/rst/index.rst:577:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.

click here for bot help

dagwieers · 2018-09-01T21:49:42Z

Can we disable those warnings ? It's not like those blocks aren't powershell.
These are false positives.

dagwieers · 2018-09-01T22:04:05Z

Looking at all the examples that hit this, all of them have backslashes in them.
It somehow seems that blackslashes are causing this ?

PS Also, the line-numbers seem to be from different files, and since index.rst seems to be a dummy name, it is very hard for contributors to understand what is going on. @felixfontein @gundalow

dagwieers · 2018-09-01T22:22:18Z

Pygments does not have any issues with the code-blocks, so it only fails through Sphinx.

dagwieers · 2018-09-01T22:39:50Z

Ok, it seems Sphinx run Pygments using -F raiseonerror as explained in sphinx-doc/sphinx#4225

And not the backslash itself seems to be the issue, but the colon is: http://pygments.org/demo/6759768/

dagwieers · 2018-09-01T22:43:54Z

And there is a fix to Sphinx here: sphinx-doc/sphinx#4250

dagwieers · 2018-09-01T23:48:16Z

Wrt. the powershell lexer, it has 2 issues with colons.

It can't handle URLs that are not quoted, like:

Set-Item -Path WSMan:\localhost\Shell\{path} -Value "value here"

The solution here is simple, quote it as a string:

Set-Item -Path "WSMan:\localhost\Shell\{path}" -Value "value here"

It can't handle option/value using a colon as the delimiter

winrs -r:http://server:5985/wsman -u:Username -p:Password ipconfig

The solution is to write it as:

winrs -r "http://server:5985/wsman" -u Username -p Password ipconfig

dagwieers · 2018-09-02T00:16:42Z

Our only true solution is to fix the lexer, but for the time being I have disabled Powershell syntax highlighting for the affected blocks. (I used "guess" which simply fails to use a lexer). I prefer doing this over working around it, so it is documented until fixed properly.

When we have a fix for the lexer, we can override the powershell lexer in our tree as we already do for the yaml+jinja lexer.

This PR includes: - Using explicit yaml+jinja code-blocks - Capital letter to start a comment or description - Use yes/no in YAML instead of true/false - Use true/false in Jinja instead of True/False - Work around pygments lexer issues with colons (in URLs and options)

jborean93

I'm not against the changes but I will leave it to @acozine as to whether we want to enforce a standard for comments/examples in the docs.

jborean93 · 2018-09-02T21:46:02Z

docs/docsite/rst/user_guide/windows_dsc.rst


-    - name: use win_dsc module with the Registry DSC resource
+    - name: Use win_dsc module with the Registry DSC resource


I'm just wondering, is there a consensus as to whether we want upper case or lower case in the docs, I treat the name block as a bullet list so don't worry about starting with caps or ending with a sentence but that's just me.

Hmm, I find it easier to read though. Especially for descriptions and documentation.

I don't mind that much for integration tests, but anything that ends up in the documentation (user-facing) I prefer to use sensible, pleasing standards.

The trouble with using this as justification is that everybody has different preferences and we could get into an endless loop with these types of edits. If @acozine has a standard that she's trying to set with the documentation then we should be following that standard but right now I'm unsure what that would be.

In general, Red Hat documentation follows the IBM Style guide. That said, it doesn't cover this scenario directly. Our Ansible style guide also doesn't cover this, but documenting modules has an example section:
https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#examples-block

That uses an initial cap. Also the few examples I poked at in https://github.com/ansible/ansible-examples also uses initial cap for names. I'll ask @acozine directly if she wants to adopt this as a standard (and say so in the docs so it's clear).

From a Community point of view we need to be careful not to cause "busy work" for Contributors and reviewers.

I think it's is OK to:

Update "how to document your module" to state how we'd like documentation to be written

update any examples in docs/docsite/rst to use the "best" form (with name:, capital letters, etc.)

Bulk update of support:core modules to use the "best" form.

If others want to update modules that's fine, though that must be the only change in the PR to make it easy to review and backport.

I wouldn't want to add review comments on PRs asking them to add capital letters (or fullstops/periods at the end).

Chatted offline w/ @acozine and she doesn't have a preference for either way. So I think the 'gist' of it is:

Don't comment on another PR to request capitalization etc change

Recommend to always use a name and make it a useful phrase or sentence.

no bulk changes to existing content, because there's no 'right or wrong' way to do it.

jborean93 · 2018-09-02T21:46:40Z

docs/docsite/rst/user_guide/windows_dsc.rst

@@ -222,10 +236,10 @@ The ``Find-DscResource`` cmdlet can also be used to find custom resources. For e

 .. code-block:: powershell

-    # find all DSC resources in the configured repositories
+    # Find all DSC resources in the configured repositories


Same thing here, is it really necessary we do this?

Is it necessary ? From a technical perspective no, but to me it's more pleasing.

But I am the guy who prefers punctuation marks and correct spelling too ;-)

jborean93 · 2018-09-02T21:49:33Z

@dagwieers you won't be able to do winrs -r "http://server:5985/wsman" -u Username -p Password ipconfig as winrs is a vbscript and require the args to be in the form currently documented. Happy for the Set-Item -Path WSMan:\... examples to be quoted though.

dagwieers · 2018-09-02T23:00:28Z

@jborean93 I am not changing any of them, I hope we can improve the lexer at some point. (However pygments seems to be unmaintained at the moment :-/)

samccann

Generated the docs locally and looks good.

samccann · 2018-11-15T20:59:10Z

Docs looks okay. @jborean93 @gundalow anything else blocking the merge here?

jborean93 · 2018-11-15T21:12:48Z

I'm ok with the technical content changes so +1 from me.

This PR includes: - Using explicit yaml+jinja code-blocks - Work around pygments lexer issues with colons (in URLs and options)

dagwieers added the windows Windows community label Sep 1, 2018

ansibot added affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 1, 2018

dagwieers force-pushed the windows-docs2 branch from 6b5c61a to 3ef9415 Compare September 1, 2018 20:55

dagwieers requested a review from jborean93 September 1, 2018 20:56

ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Sep 1, 2018

dagwieers force-pushed the windows-docs2 branch from 3ef9415 to 50a4aca Compare September 1, 2018 23:43

dagwieers requested review from acozine and gundalow September 1, 2018 23:48

ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Sep 2, 2018

dagwieers force-pushed the windows-docs2 branch from 50a4aca to a7a5145 Compare September 2, 2018 00:16

dagwieers force-pushed the windows-docs2 branch from a7a5145 to d16828a Compare September 2, 2018 17:17

jborean93 reviewed Sep 2, 2018

View reviewed changes

joaocgreis mentioned this pull request Sep 4, 2018

add yamllint nodejs/build#1476

Closed

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 10, 2018

acozine added this to To do in OLD Ansible Documentation via automation Sep 17, 2018

ansibot added the core_review In order to be merged, this PR must follow the core review workflow. label Oct 25, 2018

samccann approved these changes Nov 15, 2018

View reviewed changes

samccann merged commit fd02ecd into ansible:devel Nov 15, 2018

mjmayer pushed a commit to mjmayer/ansible that referenced this pull request Nov 30, 2018

Code example improvements in Windows documentation (ansible#45055)

789128d

This PR includes: - Using explicit yaml+jinja code-blocks - Work around pygments lexer issues with colons (in URLs and options)

acozine moved this from To do to Done in OLD Ansible Documentation Jan 28, 2019

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code example improvements in Windows documentation #45055

Code example improvements in Windows documentation #45055

dagwieers commented Sep 1, 2018 •

edited

ansibot commented Sep 1, 2018

ansibot commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 2, 2018 •

edited

jborean93 left a comment

jborean93 Sep 2, 2018

dagwieers Sep 2, 2018

jborean93 Sep 2, 2018

samccann Nov 14, 2018

gundalow Nov 15, 2018

samccann Nov 15, 2018

jborean93 Sep 2, 2018

dagwieers Sep 2, 2018

jborean93 commented Sep 2, 2018

dagwieers commented Sep 2, 2018

samccann left a comment

samccann commented Nov 15, 2018

jborean93 commented Nov 15, 2018


		- name: use win_dsc module with the Registry DSC resource
		- name: Use win_dsc module with the Registry DSC resource

Code example improvements in Windows documentation #45055

Code example improvements in Windows documentation #45055

Conversation

dagwieers commented Sep 1, 2018 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Sep 1, 2018

ansibot commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 1, 2018

dagwieers commented Sep 2, 2018 • edited

jborean93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jborean93 commented Sep 2, 2018

dagwieers commented Sep 2, 2018

samccann left a comment

Choose a reason for hiding this comment

samccann commented Nov 15, 2018

jborean93 commented Nov 15, 2018

dagwieers commented Sep 1, 2018 •

edited

dagwieers commented Sep 2, 2018 •

edited