-
Notifications
You must be signed in to change notification settings - Fork 45
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
Adding Property Group functions to the properties-service #198
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.
First glance looks good.
Can you add some tests for the new functions? There are plenty of examples in the tests folder for you to reference.
Yes, I will work on that now. Thanks :) |
Ok, just added tests into the properties-service test file. Please review and let me know if I need to make any changes to them. |
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.
Approved from my side.
@jonathanmedd Can you take a look too please?
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.
Thanks for your contribution, this is great work. Please deal with the minor comments I have made, then we can merge this.
src/Functions/Public/properties-service/Get-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/Get-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/New-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/New-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/New-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/New-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
src/Functions/Public/properties-service/Remove-vRAPropertyGroup.ps1
Outdated
Show resolved
Hide resolved
I am working on these changes now, and will have them submitted by the end of this week. Some of these I found while looking over the code after the pull request, apologies for missing them. |
Ok, I have made all requested changes and submitted as of now :) Thanks fellas. |
Hello,
this is my first pull request for this repository. I am a Consultant and as a requirement for a client I am helping now, I would like to add these three functions to interact with the Property Group endpoints of the properties-service.
Of course, please review and give me any suggested changes you may need.