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 Process Template Support #128

Merged
merged 9 commits into from
Jan 23, 2019

Conversation

fifthstreetpartners
Copy link
Contributor

@fifthstreetpartners fifthstreetpartners commented Jan 17, 2019

@DarqueWarrior
Copy link
Collaborator

The alias Get-Process is going to cause problems. I would suggest you just don't add it. Get-Process is an out of the box PowerShell cmdlet. It is very popular and we don't want to take that one over.

@DarqueWarrior
Copy link
Collaborator

My only other feedback is what should be the default format of the results to Get-VSTeamProcess? Currently it is a list and it shows the ID and URL. I am not sure why anyone would need to see that by default. I might suggest adding a table format that only shows the name and description. Users could always run Get-VSTeamProcess | FL * to get the current list view.

This is really nice work.

@fifthstreetpartners
Copy link
Contributor Author

The alias Get-Process is going to cause problems. I would suggest you just don't add it. Get-Process is an out of the box PowerShell cmdlet. It is very popular and we don't want to take that one over.

I thought about that actually but figured I'd include it just in case. No problem taking it out.

@fifthstreetpartners
Copy link
Contributor Author

My only other feedback is what should be the default format of the results to Get-VSTeamProcess? Currently it is a list and it shows the ID and URL. I am not sure why anyone would need to see that by default. I might suggest adding a table format that only shows the name and description. Users could always run Get-VSTeamProcess | FL * to get the current list view.

This is really nice work.

No problem, I'll add that.

@fifthstreetpartners
Copy link
Contributor Author

Ready for review

@DarqueWarrior
Copy link
Collaborator

Make sure you import the module and run Invoke-Pester from the Unit\Test folder. I see two tests failing.

@DarqueWarrior
Copy link
Collaborator

The tests were just missing a mock. I put them in there. Don't worry about it.

@fifthstreetpartners
Copy link
Contributor Author

Sorry I missed that test. Anything else you want me to change?

@DarqueWarrior
Copy link
Collaborator

Merging into master now. Thanks so much for your support.

@DarqueWarrior DarqueWarrior merged commit 1f75f9c into MethodsAndPractices:master Jan 23, 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.

2 participants