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

ec2_snapshot: Add check_mode support to ec2_snapshot #512

Merged
merged 13 commits into from
Oct 1, 2021
Merged

ec2_snapshot: Add check_mode support to ec2_snapshot #512

merged 13 commits into from
Oct 1, 2021

Conversation

mandar242
Copy link
Contributor

SUMMARY

Add check_mode support to ec2_snapshot.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_snapshot

@ansibullbot
Copy link

cc @jillr @s-hertel @tremble @willthames @wimnat
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Sep 24, 2021
tremble
tremble requested changes Sep 25, 2021
View reviewed changes
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.

Mostly just minor niggles. Thanks for your work on this.

tests/integration/targets/ec2_snapshot/tasks/main.yml Outdated Show resolved Hide resolved
tests/integration/targets/ec2_snapshot/tasks/main.yml Outdated Show resolved Hide resolved
plugins/modules/ec2_snapshot.py Outdated Show resolved Hide resolved
plugins/modules/ec2_snapshot.py Outdated Show resolved Hide resolved
plugins/modules/ec2_snapshot.py Outdated Show resolved Hide resolved
changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml Outdated Show resolved Hide resolved
@mandar242 mandar242 requested a review from tremble September 28, 2021 02:39
@mandar242
Copy link
Contributor Author

Mostly just minor niggles. Thanks for your work on this.

Thank you for reviewing!

jillr
jillr approved these changes Sep 30, 2021
View reviewed changes
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

LGTM, I'll leave gate off for now to give @tremble a chance to update his review for the latest changes he requested. Thanks @mandar242!

tremble
tremble requested changes Oct 1, 2021
View reviewed changes
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.

Thanks,

I'd still like to see a non-check-mode deletion idempotency test (ie deleting a deleted snapshot doesn't error), but otherwise looks good.

tests/integration/targets/ec2_snapshot/tasks/main.yml Show resolved Hide resolved
tremble
tremble approved these changes Oct 1, 2021
View reviewed changes
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.

LGTM. Let's get this gated!

@tremble tremble closed this Oct 1, 2021
@tremble tremble reopened this Oct 1, 2021
@tremble tremble added the gate label Oct 1, 2021
@tremble
Copy link
Contributor

tremble commented Oct 1, 2021

check had passed, I accidentally closed the PR.

@tremble
Copy link
Contributor

tremble commented Oct 1, 2021

Check buildset: https://dashboard.zuul.ansible.com/t/ansible/buildset/a2cfdceedb3d442a889a32087b755dd5

ansible-zuul[bot]
ansible-zuul bot approved these changes Oct 1, 2021
View reviewed changes
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@ansible-zuul ansible-zuul bot merged commit 33e21ad into ansible-collections:main Oct 1, 2021
@mandar242 mandar242 deleted the ec2_snapshot_check_mode_support branch October 4, 2021 17:18
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
@tremble
move changelog into the right place (ansible-collections#512)
13f7a0e 
ansible-collections#302 landed a changelog fragment into the wrong place, fix it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ansible-zuul ansible-zuul[bot] ansible-zuul[bot] approved these changes

@jillr jillr jillr approved these changes

@tremble tremble tremble approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@mandar242 @ansibullbot @tremble @jillr