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

vmware_object_custom_attributes_info: Add a new module to gather custom attributes of an object #851

Merged
merged 10 commits into from
May 26, 2021
Merged

vmware_object_custom_attributes_info: Add a new module to gather custom attributes of an object #851

merged 10 commits into from
May 26, 2021

Conversation

sky-joker
Copy link
Collaborator

@sky-joker sky-joker commented May 19, 2021 •
edited
Loading

SUMMARY

This PR will add a new module to gather custom attributes of an object.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

README.md
changelogs/fragments/851-vmware_object_custom_attributes_info.yml
docs/community.vmware.vmware_object_custom_attributes_info_module.rst
meta/runtime.yml
plugins/modules/vmware_object_custom_attributes_info.py
tests/integration/targets/vmware_object_custom_attributes_info/aliases
tests/integration/targets/vmware_object_custom_attributes_info/tasks/main.yml
tests/integration/targets/vmware_object_custom_attributes_info/tasks/pre.yml
tests/integration/targets/vmware_object_custom_attributes_info/tasks/vmware_object_custom_attributes_info_tests.yml
tests/sanity/ignore-2.9.txt
tests/sanity/ignore-2.10.txt
tests/sanity/ignore-2.11.txt
tests/sanity/ignore-2.12.txt

ADDITIONAL INFORMATION

tested on vCenter 7.0

@sky-joker sky-joker changed the title [WIP]vmware_object_custom_attributes_info: Add a new module to gather custom attributes of an object vmware_object_custom_attributes_info: Add a new module to gather custom attributes of an object May 20, 2021
mariolenz
mariolenz requested changes May 21, 2021
View reviewed changes
Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

Use FQCN for examples within module documentation and in integration tests. Rest LGTM!

Thanks @sky-joker for this awsome new module! I've tested it in our environment and it works like a charm.

plugins/modules/vmware_object_custom_attributes_info.py Outdated Show resolved Hide resolved
tests/integration/targets/vmware_object_custom_attributes_info/tasks/pre.yml Outdated Show resolved Hide resolved
tests/integration/targets/vmware_object_custom_attributes_info/tasks/pre.yml Outdated Show resolved Hide resolved
...ts/vmware_object_custom_attributes_info/tasks/vmware_object_custom_attributes_info_tests.yml Outdated Show resolved Hide resolved
...ts/vmware_object_custom_attributes_info/tasks/vmware_object_custom_attributes_info_tests.yml Outdated Show resolved Hide resolved
Akasurde
Akasurde requested changes May 21, 2021
View reviewed changes
Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

Apart from @mariolenz and my comments, LGTM

plugins/modules/vmware_object_custom_attributes_info.py Outdated
description:
- This module can be gathered custom attributes of an object.
requirements:
- python >= 2.7
Copy link
Member

Choose a reason for hiding this comment

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

Since Python 2 is EOL, we should start testing and using Python 3 only code. and set minimum requirement to Python 3.

plugins/modules/vmware_object_custom_attributes_info.py Show resolved Hide resolved
@sky-joker
Copy link
Collaborator Author

Thanks, @mariolenz @Akasurde for reviewing.
I reflected on your feedback, so could you please look at this one again?

@ansibullbot
Copy link

cc @Tomorrow9 @goneri @lparkes @pgbidkar @warthog9
click here for bot help

@ansibullbot ansibullbot added cloud community_review docs feature This issue/PR relates to a feature request integration tests/integration module module needs_triage Needs a first human triage before being processed. new_module New module new_plugin New plugin plugins plugin (any type) tests tests labels May 22, 2021
@mariolenz
Copy link
Collaborator

Thanks @sky-joker, LGTM now!

@sky-joker sky-joker requested a review from Akasurde May 23, 2021 09:32
@mariolenz
Copy link
Collaborator

@Akasurde You've requested changes to this PR and @sky-joker pushed a new commit. What do you think, are your change requests addressed by this commit? Would it be OK to merge or do you see something that still needs changes?

I've already approved, so if you think this PR is OK now feel free to add the gate label.

Akasurde
Akasurde approved these changes May 26, 2021
View reviewed changes
Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

LGTM

@Akasurde Akasurde added gate and removed needs_triage Needs a first human triage before being processed. labels May 26, 2021
@ansible-zuul ansible-zuul bot merged commit 70d752b into ansible-collections:main May 26, 2021
@sky-joker
Copy link
Collaborator Author

Thanks, @Akasurde @mariolenz for looking at this one!

@sky-joker sky-joker deleted the add_object_custom_attribute_info_module branch May 26, 2021 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariolenz mariolenz mariolenz approved these changes

@Akasurde Akasurde Akasurde approved these changes

@goneri goneri Awaiting requested review from goneri

Assignees
No one assigned
Labels
cloud docs feature This issue/PR relates to a feature request integration tests/integration module module new_module New module new_plugin New plugin plugins plugin (any type) shipit tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sky-joker @ansibullbot @mariolenz @Akasurde