-
Notifications
You must be signed in to change notification settings - Fork 324
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
Cleanup additional resources during uninstall #820
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most excellent work!!
I had one additional thought, and we don't usually require negative testing, but given this is deleting things would it make sense to add a test where we pass in a different release or namespace to ensure that our code is not actually deleting other people's stuff? If that doesnt make sense I'd be happy to explain :)
Co-authored-by: Kyle Schochenmaier <kschoche@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Resolved the CHANGELOG conflict, should we go ahead and merge @NicoletaPopoviciu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work on this, thanks so much, it looks great!
Changes proposed in this PR:
How I've tested this PR:
How I expect reviewers to test this PR:
Checklist: