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.

Sign up for GitHub

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

Jump to bottom

Enhance Get-VSTeamProcess #300 #322

Closed
wants to merge 10 commits into from
Closed

Enhance Get-VSTeamProcess #300 #322

wants to merge 10 commits into from

Conversation

jhoneill
Copy link
Contributor

PR Summary

Get-VSTeamProcess now:

  • Gets information from the /work/processes URI path rather than process/processes as the former has additional details
  • Can query for processes using a wildcard name.
  • Updates when it runs the cached process information
  • Displays whether a process was disabled or is the default process.

PR Checklist

  • [Write Help]
    (Small updates, mostly reflecting the different API provides more information but doesn't support paged responses)
  • [Write Unit Test]
    (Fixed existing ones.)
  • [Update CHANGELOG.md]

DarqueWarrior
DarqueWarrior requested changes May 23, 2020
View reviewed changes
Copy link
Collaborator

@DarqueWarrior DarqueWarrior left a comment

Choose a reason for hiding this comment

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

I do not want to change the build process in any way at this time.
I do not want any testing flags of any kind.

.docs/Get-VSTeamProcess.md Show resolved Hide resolved
Build-Module.ps1 Outdated Show resolved Hide resolved
@jhoneill
Copy link
Contributor Author

jhoneill commented May 24, 2020 •
edited
Loading

I do not want to change the build process in any way at this time.

I've changed my build so that

  • _Classes.json is as as you had it and vsteam.classes.ps1 gets built.
  • vsteam.psm1 dot sources vsteam.classes.ps1 as you had it
  • vsteam.psm1 is copied as is to the output directory, as you had it.

I have also added a switch to build module if specified

  • vsteam.psm1 in the output directory is appended to vsteam.classes.ps1
  • the now-extended vsteam.classes.ps1 is copied over vsteam.psm1.
  • vsteam.classes.ps1 will still be dot sourced so is replaced with "#Empty"

So have we can development that little bit easier with a build-time switch and the "normal" build process is unaffected.

I do not want any testing flags of any kind.

I found one last stray one in Add-VSTeamBuild.ps1 they are all removed from my branches now (I think)
_edit / postscript I removed it wrong and broke the test. Doh! Fortunately I tested before pushing the commit.

DarqueWarrior
DarqueWarrior requested changes May 25, 2020
View reviewed changes
Copy link
Collaborator

@DarqueWarrior DarqueWarrior left a comment

Choose a reason for hiding this comment

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

Would be nice to apply the Update -List pattern to all cache classes.

Build-Module.ps1 Show resolved Hide resolved
.docs/Get-VSTeamProcess.md Outdated Show resolved Hide resolved
.docs/Get-VSTeamProcess.md Outdated Show resolved Hide resolved
Source/Public/Get-VSTeamProcess.ps1 Outdated Show resolved Hide resolved
DarqueWarrior
DarqueWarrior reviewed Jun 10, 2020
View reviewed changes
Source/Private/common.ps1
@@ -154,63 +143,18 @@ function _getApiVersion {
[CmdletBinding(DefaultParameterSetName = 'Service')]
param (
[parameter(ParameterSetName = 'Service', Mandatory = $true, Position = 0)]
[ValidateSet('Build', 'Release', 'Core', 'Git', 'DistributedTask', 'VariableGroups', 'Tfvc', 'Packaging', 'MemberEntitlementManagement', 'ExtensionsManagement', 'ServiceFabricEndpoint', 'Graph', 'TaskGroups', 'Policy')]
[ValidateScript({$_ -in ([VSTeamVersions] | Get-Member -Static -MemberType Property).name})]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Versions has more values like Account, DefaultProject and Version that are not valid for Service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although they are not services, they can still be returned. If it's necessary to throw if someone uses _getApiVersion to find the account that can be done.
However I am adding support for managing processes definitions which really needed its own API version (possibly not with the most recent changes) and having to add to this list in common.ps1 when a change is made to class definition felt wrong.

@DarqueWarrior
Copy link
Collaborator

How are you merging the changes from upstream? I see 235 files changed but they really didn't it is making it very time consuming to review this PR.

@jhoneill
Copy link
Contributor Author

How are you merging the changes from upstream? I see 235 files changed but they really didn't it is making it very time consuming to review this PR.

It was some time back... I think I pulled upstream into to my master branch and then merged master into this branch which had already been created for the PR.

@DarqueWarrior
Copy link
Collaborator

I should have these changes in today.

@DarqueWarrior
Copy link
Collaborator

Switching Get-VSTeamProcess from Processes to Work breaks TFS2017. I am working switching the call based on the API version being used. I will be merging these changes from another PR because there are too many files changed in this PR to make sense of the actual changes.

DarqueWarrior added a commit that referenced this pull request Jul 13, 2020
@DarqueWarrior
Review of PR 322 (#340)
1ccb9de 
Pulled in changes from PR #322 
Added Timeout setting
Integration tests are now safe to run in any account. They are not longer destructive.

Co-authored-by: James O'Neill <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarqueWarrior DarqueWarrior DarqueWarrior requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jhoneill @DarqueWarrior