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

Add new semantic markup support to ansible-doc text output and to validate-modules #74937

Closed
wants to merge 4 commits into from
Closed

Add new semantic markup support to ansible-doc text output and to validate-modules #74937

wants to merge 4 commits into from

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Implements ansible-doc part of #73137.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ansible-doc

@felixfontein felixfontein mentioned this pull request Jun 8, 2021
Closed
@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.12 feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 8, 2021
This was referenced Jun 8, 2021
Closed
Merged
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Jun 10, 2021
@felixfontein felixfontein mentioned this pull request Jun 15, 2021
Closed
@acozine acozine mentioned this pull request Jun 18, 2021
Closed
6 tasks
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. has_issue labels Jun 18, 2021
bcoca
bcoca reviewed Jun 22, 2021
View reviewed changes
lib/ansible/cli/doc.py Outdated
_LINK = re.compile(r"\bL\(([^)]+), *([^)]+)\)")
_URL = re.compile(r"\bU\(([^)]+)\)")
_REF = re.compile(r"\bR\(([^)]+), *([^)]+)\)")
_CONST = re.compile(r"\bC\(([^)]+)\)")
_SEM_OPTION_NAME = re.compile(r"\bO\(([^)]+)\)")
_SEM_OPTION_VALUE = re.compile(r"\bV\(([^)]+)\)")
_SEM_ENV_VARIABLE = re.compile(r"\bE\(([^)]+)\)")
Copy link
Member

Choose a reason for hiding this comment

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

i would make these 3 into one (since they are basically same behavior)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Combining C(), V() and E() makes senes, while I would keep O() differently. In the form O() without = in the parameter, the formatting should be bold (according to the IBM style guide), so if this is combined with your colors PR, this should be handled differently.

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 25, 2021
@samccann samccann mentioned this pull request Sep 9, 2021
Closed
1 task
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Oct 9, 2021
@ansibot 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 Oct 19, 2021
@ansibot ansibot removed 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 Nov 21, 2021
@samccann samccann mentioned this pull request Dec 21, 2021
Closed
@ansibot 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 Dec 21, 2021
@ansibot ansibot removed 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 Dec 24, 2021
@felixfontein felixfontein changed the title [WIP] Add new semantic markup support to ansible-doc text output [WIP] Add new semantic markup support to ansible-doc text output and to validate-modules Dec 24, 2021
@ansibot ansibot added the test This PR relates to tests. label Dec 24, 2021
@felixfontein felixfontein mentioned this pull request Dec 24, 2021
Closed
@ansibot 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 Dec 10, 2022
@ansibot ansibot removed 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 Feb 25, 2023
@felixfontein felixfontein mentioned this pull request Mar 1, 2023
Closed
@felixfontein felixfontein changed the title [WIP] Add new semantic markup support to ansible-doc text output and to validate-modules Add new semantic markup support to ansible-doc text output and to validate-modules Mar 9, 2023
@felixfontein felixfontein marked this pull request as ready for review March 9, 2023 21:23
@felixfontein
Copy link
Contributor Author

I rebased to latest devel. The community approved the plan of trying to get semantic markup into ansible-core (see ansible-community/community-topics#53 (comment)), so I'm marking this as:

ready_for_review

🎉

@ansibot ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Mar 9, 2023
@mattclay mattclay mentioned this pull request Mar 14, 2023
Merged
@mattclay
Copy link
Member

@felixfontein Can you add some integration tests to cover this new functionality? The ansible-test-sanity-validate-modules and ansible-doc targets are probably the best places for the tests.

@ansibot ansibot added the new_plugin This PR includes a new plugin. label Mar 14, 2023
@samccann
Copy link
Contributor

@mattclay @bcoca - since this is part of semantic markup support, do you feel this will be merged before 2.15 code freeze?

@felixfontein
Copy link
Contributor Author

I can also split this up into two PRs - one for ansible-doc and one for validate-modules - if that helps.

@felixfontein felixfontein mentioned this pull request Mar 15, 2023
Merged
@bcoca
Copy link
Member

bcoca commented Mar 16, 2023

@samccann it is not waiting on us
@felixfontein probably, I can see us merging the ansible-doc changes w/o having impact on others, validate-modules part on the other hand ...

This was referenced Mar 16, 2023
Merged
Merged
@felixfontein
Copy link
Contributor Author

I created #80242 with the ansible-doc changes, and #80243 with the validate-modules changes. I can see whether I can come up with a more minimalistic validate-modules PR that at least avoids syntax errors for the new seealsos, but that doesn't check semantic markup. Would be a pitty to miss the best parts, but ...

@felixfontein felixfontein mentioned this pull request Mar 16, 2023
Merged
@felixfontein
Copy link
Contributor Author

#80244 contains only the seealso schema change for validate-modules. That is the bare minimum of semantic markup support for validate-modules that is needed (the markup itself is not validated) so that one doesn't need to add ignore.txt entries when using this part of semantic markup.

@felixfontein
Copy link
Contributor Author

Closing in favor of #80242 and #80243 or at least #80244.

@felixfontein felixfontein deleted the semantic-markup branch March 20, 2023 19:38
@ansible ansible locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bcoca bcoca bcoca left review comments

Assignees
No one assigned
Labels
affects_2.12 feature This issue/PR relates to a feature request. has_issue new_plugin This PR includes a new plugin. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@felixfontein @mattclay @samccann @bcoca @ansibot @jborean93