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

Added DetailedObject parameter to Get-RubrikVM #264

Merged
merged 4 commits into from
Mar 18, 2019
Merged

Conversation

jaapbrasser
Copy link
Contributor

Description

Get-RubrikVM inconsistent results

Related Issue

#262

Please link to the issue here

Motivation and Context

Currently Get-RubrikVM -Name and Get-RubrikVM -id return different datasets, which can be confusing from a user perspective. I added the -DetailedObject parameter, to execute an additional query by ID to retrieve the full details on the computer object.

How Has This Been Tested?

I created an if-else statement that will execute a second query. I tested this code against the TM lab environment

Screenshots (if appropriate):

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

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!

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTION document.
  • I have updated the CHANGELOG file accordingly for the version that this merge modifies.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jaapbrasser
Copy link
Contributor Author

I added an additional parameter -DetailedObject, If anyone has a suggestion for a different parameter name I would be happy to update it. The idea of the change is to prevent the unintuative approach of having to pipe Get-RubrikVM myvm01 into Get-RubrikVM in order to get full details. The switch parameter abstracts this away for the user to give consistent results.

Copy link
Contributor

@chriswahl chriswahl left a comment

Choose a reason for hiding this comment

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

This is a good idea. It's not fun having to request the VM details a second time. I just made one small suggestion before merging. 👍

@@ -27,6 +27,10 @@ function Get-RubrikVM
.EXAMPLE
Get-RubrikVM -Relic
This will return all removed virtual machines that were formerly protected by Rubrik.

.EXAMPLE
Get-RubrikVM -id myserver01 -DetailedObject
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this example should use the Name param.

Get-RubrikVM -Name 'Server1' -DetailedObject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool I updated the description and added a progress bar for DetailedObject queries

@jaapbrasser jaapbrasser requested a review from chriswahl March 17, 2019 23:46
Copy link
Contributor

@chriswahl chriswahl left a comment

Choose a reason for hiding this comment

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

👍 👍

@jaapbrasser jaapbrasser merged commit 7089c77 into master Mar 18, 2019
@jaapbrasser jaapbrasser deleted the jaapjaapdev branch March 18, 2019 00:18
@jaapbrasser jaapbrasser self-assigned this Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants