-
Notifications
You must be signed in to change notification settings - Fork 87
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
Add Remove-RubrikVMSnapshot cmdlet and additional logging #308
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.
Everything looks great to me - I will reassign to Jaap for final call. Good job!
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.
Looks good @shamsway, thanks for the last minute changes
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.
👍
Resolve #148 |
Description
This PR adds a new
Remove-RubrikVMSnapshot
cmdlet and associated unit test, along with some additional verbose logging. It also modifies the logic so thatDELETE
API calls return a response when the API call is successful. Previously the user was given no response in this scenario.Related Issue
This project only accepts pull requests related to open issues.
#148
Motivation and Context
Why is this change required? What problem does it solve?
This PR addresses #148, and it provides additional useful output to the user
How Has This Been Tested?
Tested against CDM 5.0.1 from MacOS. I tested almost all of the
Remove-*
cmdlets to verify the new response would not affect them, and tested severalGet-*
cmdlets as a smoke test to verify that the code needed for additional verbose logging did not have unintentional effects.Screenshots (if appropriate):
Types of changes
What types of changes does your code introduce? Put an
x
in all the boxes that apply:Checklist:
Go over all the following points, and put an
x
in all the boxes that apply. If you're unsure about any of these, don't hesitate to ask. We're here to help!