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

inventory/aws2_ec2: avoid extra Display import #226

Conversation

goneri
Copy link
Member

@goneri goneri commented Jan 5, 2021

The BaseInventoryPlugin class already expose an instance of
ansible.utils.display.Display. We don't to recreate it.

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

The superclass does appear to include display in self, which is imported identically to the way it had been in this module.

https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/inventory/__init__.py#L39
/ https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/inventory/__init__.py#L161

Change looks sane and tests pass

@tremble
Copy link
Contributor

tremble commented Jan 7, 2021

@jillr / @felixfontein Do you feel this change warrants a changelog entry?

@felixfontein
Copy link
Contributor

@tremble good question. I tend to say "all code changes need one", to stop having to think about such cases ;-)

The `BaseInventoryPlugin` class already expose an instance of
`ansible.utils.display.Display`. We don't to recreate it.
@goneri goneri force-pushed the inventory-aws2_ec2-avoid-extra-Display-import_2514 branch from 559caf2 to d53dd6a Compare January 7, 2021 16:54
@goneri goneri requested a review from tremble January 7, 2021 17:14
Co-authored-by: Mark Chappell <mchappel@redhat.com>
@goneri goneri requested a review from tremble January 7, 2021 20:35
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

Co-authored-by: Felix Fontein <felix@fontein.de>
@tremble
Copy link
Contributor

tremble commented Jan 8, 2021

Thanks for tweaking the Changelog @goneri and @Akasurde

@tremble tremble merged commit ab3cea9 into ansible-collections:main Jan 8, 2021
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…_protocol_versions (ansible-collections#226)

Co-authored-by: Andreas Jonsson <andreas.jonsson@ensighten.com>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…_protocol_versions (ansible-collections#226)

Co-authored-by: Andreas Jonsson <andreas.jonsson@ensighten.com>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…_protocol_versions (ansible-collections#226)

Co-authored-by: Andreas Jonsson <andreas.jonsson@ensighten.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants