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

validate-modules: Fix net_tools modules #52489

Merged
merged 1 commit into from
Feb 25, 2019
Merged

validate-modules: Fix net_tools modules #52489

merged 1 commit into from
Feb 25, 2019

Conversation

dagwieers
Copy link
Contributor

SUMMARY

This PR includes:

  • Fixes to open validate-modules issues
  • Adding parameter types
ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

net_tools modules

@ansibot
Copy link
Contributor

ansibot commented Feb 18, 2019

cc @MeganLiu @andreaso @jpmens @jtyr @mattclay @mgruener @mpdehaan @nerzhul @ogenstad @ptux @ravibhure @resmo @romeotheriault
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. net_tools Net-tools category support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Feb 18, 2019
@jpmens
Copy link
Contributor

jpmens commented Feb 18, 2019

lgtm

@dagwieers
Copy link
Contributor Author

The CI issues are to be expected as we are adding parameters missing from the documentation.

@ansibot

This comment has been minimized.

Sign in to view
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. labels Feb 18, 2019
@dagwieers dagwieers force-pushed the validate_modules-net_tools branch 2 times, most recently from 13bfe54 to 4781ee9 Compare February 18, 2019 12:32
@ansibot

This comment has been minimized.

Sign in to view
@dagwieers
Copy link
Contributor Author

We have a fix for the sanity tests for this issue at: #52497

acozine
acozine reviewed Feb 18, 2019
View reviewed changes
Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

Overall the docs changes look good - I made some suggestions.

I'm not sure how to evaluate some of the changes here - it would be great to get more eyes on this PR.

lib/ansible/modules/net_tools/basics/uri.py Outdated Show resolved Hide resolved
lib/ansible/modules/net_tools/basics/get_url.py Show resolved Hide resolved
lib/ansible/modules/net_tools/basics/uri.py Show resolved Hide resolved
lib/ansible/modules/net_tools/basics/uri.py Outdated Show resolved Hide resolved
lib/ansible/modules/net_tools/basics/uri.py Outdated Show resolved Hide resolved
lib/ansible/modules/net_tools/ldap/ldap_attr.py Show resolved Hide resolved
lib/ansible/modules/net_tools/omapi_host.py Show resolved Hide resolved
lib/ansible/modules/net_tools/omapi_host.py Show resolved Hide resolved
lib/ansible/modules/net_tools/snmp_facts.py Show resolved Hide resolved
lib/ansible/modules/net_tools/snmp_facts.py Show resolved Hide resolved
@dagwieers
Copy link
Contributor Author

dagwieers commented Feb 18, 2019 •
edited

@acozine I did not indicate this as a Bugfix Pull Request or a Feature Pull Request, because it does not consist of bug fixes or features.

The module that included potentially dangerous changes was moved into its own PR #52493.

@dagwieers dagwieers added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 18, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Feb 18, 2019
@ansibot

This comment has been minimized.

Sign in to view
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Feb 18, 2019
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 20, 2019
@ansibot
Copy link
Contributor

ansibot commented Feb 21, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/net_tools/basics/get_url.py:0:0: E309 version_added for new option (http_agent) should be '2.8'. Currently StrictVersion ('0.0')
lib/ansible/modules/net_tools/basics/uri.py:0:0: E309 version_added for new option (http_agent) should be '2.8'. Currently StrictVersion ('0.0')
lib/ansible/modules/net_tools/ldap/ldap_attr.py:0:0: E309 version_added for new option (params) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Feb 22, 2019
@dagwieers
validate-modules: Fix net_tools modules
db07feb 
This PR includes:

* Fixes to open validate-modules issues
* Adding parameter types
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Feb 25, 2019
@dagwieers dagwieers merged commit 4670e41 into ansible:devel Feb 25, 2019
@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@acozine acozine acozine left review comments

@gundalow gundalow gundalow approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. net_tools Net-tools category support:community This issue/PR relates to code supported by the Ansible community. 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.

None yet
6 participants
@dagwieers @ansibot @jpmens @acozine @gundalow @mattclay