Uh oh!
There was an error while loading. Please reload this page.
- Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing… - #474
- Added Update-VSTeamGitRepositoryDefaultBranch to allow for changing…#474rbleattler wants to merge 4 commits into
Conversation
… the default branch of a repository. - Fixed bad link in README.md
SebastianSchuetze
left a comment
There was a problem hiding this comment.
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!
| @@ -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.
| - 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.
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.
Could you explain this to me?
There was a problem hiding this comment.
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.
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.
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.
Cleaning up, and re-committing.
| ## Assert | ||
| Should -Invoke Invoke-RestMethod -ParameterFilter { | ||
| Write-Debug "Method : $Method" |
There was a problem hiding this comment.
Mhh this works? Never thought of that!
There was a problem hiding this comment.
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.
| # 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.
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.
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.
Fixed, re-committing.
| $body = @{ | ||
| # name = $Name |
There was a problem hiding this comment.
Also remove as soon as it is clear what you wanna do.
There was a problem hiding this comment.
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
rbleattler
commented
Aug 18, 2022
Closing and re-opening with proposed changes. |
PR Summary
PR Checklist