-
Notifications
You must be signed in to change notification settings - Fork 155
- Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing… #474
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
Conversation
… the default branch of a repository. - Fixed bad link in README.md
SebastianSchuetze
left a comment
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.
Hey @rbleattler thank you for the contribution!
After reviewing it feels like you are not completely sure about your approach with some parts of the code. We need to clarify and clean this up before we can merge it!
CHANGELOG.md
Outdated
| @@ -1,5 +1,8 @@ | |||
| # Changelog | |||
|
|
|||
| - Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing the default branch of a repository. | |||
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.
| - Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing the default branch of a repository. | |
| ## 7.8.0 | |
| - Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing the default branch of a repository. |
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.
For a new freature you can raise the minor version. Can you please do the same in the PS manifest?
| # Storing the object before you return it cleaned up the pipeline. | ||
| # When I just write the object from the constructor each property | ||
| # seemed to be written | ||
| # $repo = [vsteam_lib.GitRepository]::new($resp, $ProjectName) |
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.
Could you explain this to me?
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.
This was a remnant from the function I used as a basis for creating this. Will remove and re-commit.
| # $Method -eq 'Patch' -and $Uri -eq "https://localhost:8080/tfs/PeopleTracker/PeopleTracker/_apis/git/repositories/00000000-0000-0000-0000-000000000000?api-version=$(_getApiVersion Git)" | ||
| # } | ||
| # } | ||
| # } |
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.
Please remove it if not needed. Or are you not finished yet?
Or do you try to solve something specific?
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.
I was trying to follow convention used in a different test but am not particularly proficient with pester and wanted to get this out there. Apologies for not removing it.
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.
Cleaning up, and re-committing.
|
|
||
| ## Assert | ||
| Should -Invoke Invoke-RestMethod -ParameterFilter { | ||
| Write-Debug "Method : $Method" |
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.
Mhh this works? Never thought of that!
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.
The debug lines (meant to remove, again sorry -- working on this at work while working on some migration stuff so in a time crunch) or the assertion itself?
| # area : git | ||
| # resourceName : repositories | ||
| # routeTemplate : {project}/_apis/{area}/{resource}/{repositoryId} | ||
| # http://bit.ly/Add-VSTeamGitRepository |
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.
| # http://bit.ly/Add-VSTeamGitRepository | |
| # http://bit.ly/Add-VSTeamGitRepository |
Please update this to the same name as the cmdlet
| ) | ||
| begin { | ||
| try { | ||
| # $Repo = Get-VSTeamGitRepository -Name $Name -ProjectName $ProjectName |
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.
Why are you not using this cmdlet? We are fine to use existing cmdlet to make more complex ones.
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.
Honestly, I believe I intended to be revert this once the tests were working, but this is related to the aforementioned issue with Pester.
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.
Fixed, re-committing.
|
|
||
|
|
||
| $body = @{ | ||
| # name = $Name |
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.
Also remove as soon as it is clear what you wanna do.
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.
Cleaning up, and re-committing.
…ze it as something when mocked) Removed call to dot source common.ps1 Removed debug messages Cleaned up comments Bumped Version Updated Changelog
|
Closing and re-opening with proposed changes. |
PR Summary
PR Checklist