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

support deleting vpcs that are non-default #243

Merged
merged 3 commits into from
Nov 17, 2021

Conversation

ekristen
Copy link
Contributor

This takes the existing VPC code and implements it for non-default per region nuking.

Copy link
Contributor

@bwhaley bwhaley left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

One request: could you please add VPCs to the list of resources in the README, and also add a subsection under Usage that explains that that default VPCs are excluded when nuking VPCs? I agree with you that excluding them makes sense since we have a dedicated defaults-aws command for that. Also, nuking the default VPCs would break some internal use cases we have. But we ought to explain our reasoning in the docs.

Thanks!

@ekristen
Copy link
Contributor Author

@bwhaley changes made as requested.

Copy link
Contributor

@bwhaley bwhaley left a comment

Choose a reason for hiding this comment

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

Thanks. One fix and I think this is ready to ship.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Ben Whaley <[email protected]>
Copy link
Contributor

@bwhaley bwhaley left a comment

Choose a reason for hiding this comment

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

LGTM!

@bwhaley bwhaley merged commit e3a9c93 into gruntwork-io:master Nov 17, 2021
@bwhaley
Copy link
Contributor

bwhaley commented Nov 17, 2021

@ekristen ekristen deleted the vpcs branch November 17, 2021 00:23
@ekristen
Copy link
Contributor Author

Thanks @bwhaley I've got a number of other PRs open ;-) -- I'll make sure to update the README on each one too.

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.

2 participants