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

Add Get-VSTeamGroup / Get-VSTeamDescriptor #140

Merged
merged 12 commits into from
Feb 16, 2019
Merged

Add Get-VSTeamGroup / Get-VSTeamDescriptor #140

merged 12 commits into from
Feb 16, 2019

Conversation

MichelZ
Copy link
Contributor

@MichelZ MichelZ commented Feb 12, 2019

PR Summary

Add Get-VSTeamGroup / Get-VSTeamDescriptor

I'm looking to get this PR through before I add more commands related to this (e.g. New-VSTeamGroup ;) , Add-VSTeamGroupMember, Set-VSTeamAccessControlEntry, things like that), but I'd like to see if my contribution is up to the standards first.

Also, I seem to have (at least?) 1 failing Test, which I somehow cannot figure out... A little hint would be appreciated 🥇

Executing script C:\MIZE\DEV\GitHub\vsteam\unit\test\groups.Tests.ps1

Describing Groups VSTS Context Get-VSTeamGroup by project [-] Should return groups 122ms Expected Invoke-RestMethod in module VSTeam to be called 1 times exactly but was called 0 times 34: Assert-MockCalled Invoke-RestMethod -Exactly 1 -ParameterFilter { at <ScriptBlock>, C:\MIZE\DEV\GitHub\vsteam\unit\test\groups.Tests.ps1: line 34

The Url looks exactly the same when I execute with -Verbose in a normal PS window... :(

Thanks!

PR Checklist

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 12, 2019

Also, the other Unit Tests seem to work on my system, and I can't figure out what I'm doing wrong. I'm sure you can take a quick look and spot it immediately.. thanks! 👍

@DarqueWarrior
Copy link
Collaborator

I will review today.

@DarqueWarrior
Copy link
Collaborator

All the tests are passing now. Take a look at the changes I made to make sure they are in line with what you were going for. I would like to get all the commented out code either removed for working before we merge to master.

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 14, 2019

Thanks! Yes, the changes make sense, and I understand it now, perfect.
I'll give it another try to filter by SubjectTypes, or remove the code if I can't get it to work. Thanks for the help!

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 14, 2019

Found out how to use SubjectTypes, pushed changes with Docs and new Unit Tests.

@MichelZ
Copy link
Contributor Author

MichelZ commented Feb 16, 2019

@DarqueWarrior
Are you waiting on anything from me to continue this merge?

@DarqueWarrior
Copy link
Collaborator

Sorry for the delay I am on a work trip. I am looking into now.

@DarqueWarrior DarqueWarrior merged commit e6a007a into MethodsAndPractices:master Feb 16, 2019
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.

3 participants