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.

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

simplify module argspec vs doc type mismatch checks, display - by default #58646

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from

Conversation

@bcoca
Copy link
Member

@bcoca bcoca commented Jul 2, 2019

simplify checks for type on modules, only care if they do not match
fixes #58645

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME

docs/tests

@ansibot
Copy link
Contributor

@ansibot ansibot commented Jul 2, 2019

test/sanity/validate-modules/main.py Outdated Show resolved Hide resolved
test/sanity/validate-modules/main.py Outdated Show resolved Hide resolved
@ansibot

This comment has been hidden.

@bcoca bcoca force-pushed the use_the_defaults branch from 09b18ca to bec1c28 Jul 9, 2019
@ansibot

This comment has been hidden.

@ansibot

This comment has been hidden.

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Nov 22, 2019

I've rebased after #59060 was merged.

@ansibot
Copy link
Contributor

@ansibot ansibot commented Dec 26, 2019

bcoca and others added 6 commits Feb 1, 2020
  simplify checks for type on modules, only care if they do not match
Co-Authored-By: Felix Fontein <felix@fontein.de>
Update ignore.txt.

remove dupes

updated as per meeting, default to -, not string

remove dupes

removed

now pass
doc_type = doc_options_arg.get('type', 'str')
arg_type = data.get('type', 'str')
if doc_type is None:
doc_type = 'str'
Copy link
Contributor

@felixfontein felixfontein Feb 1, 2020

I'm not sure whether we should really do that. There are still too many modules which don't declare their types, so as a user (with module development background) I'm way more happy if str is explicitly mentioned in the documentation.

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Feb 2, 2020

(Tests are currently failling because AIX instances aren't starting. Should be completely unrelated to this PR.)

Copy link
Contributor

@samccann samccann left a comment

docs portion LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants