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

GitHubBranches: Add New/Remove-GitHubRepositoryBranch Functions #256

Merged
merged 25 commits into from
Jul 16, 2020

Conversation

X-Guardian
Copy link
Contributor

Description

This PR adds the following functions to the GitHubBranches module:

  • New-GitHubRepositoryBranch
  • Remove-GitHubRepositoryBranch

Issues Fixed

None

References

GitHub Repos API

Checklist

  • You actually ran the code that you just wrote, especially if you did just "one last quick change".
  • Comment-based help added/updated, including examples.
  • Static analysis is reporting back clean.
  • New/changed code adheres to our coding guidelines.
  • Changes to the manifest file follow the manifest guidance.
  • Unit tests were added/updated and are all passing. See testing guidelines.
  • Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

@HowardWolosky HowardWolosky added api completeness This is basic API functionality that hasn't been implemented yet. api-refs Work to complete the API's defined here: https://developer.github.com/v3/git/refs/ labels Jun 29, 2020
@X-Guardian
Copy link
Contributor Author

I think it probably makes more sense to allow #97 to go in, and then have this function be a wrapper on top of New-GitHubReference. You can still make this an intelligent helper that can automate retrieving the sha via an internal call to Get-GitHubRepositoryBranch when it hasn't been passed in manually or via the pipeline, but given that we'll want New-GitHubReference in the module anyway to be able to support tag references, it seems like it makes more sense to not duplicate the logic of forming a call to create new references across two functions. Same for Remove-*.

If #97 is still a little way to being complete, how about merging this as-is and then converting it to use the reference function when it is available?

@X-Guardian X-Guardian marked this pull request as ready for review June 29, 2020 17:03
@HowardWolosky
Copy link
Member

If #97 is still a little way to being complete, how about merging this as-is and then converting it to use the reference function when it is available?

Totally reasonable idea. I'll read through this again in that light for any additional feedback, and if there isn't any, will get it merged in soon.

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

This looks like it's pretty close to merging in.
Just a few more things to touch up.
Thanks!

GitHubBranches.ps1 Show resolved Hide resolved
GitHubBranches.ps1 Outdated Show resolved Hide resolved
GitHubBranches.ps1 Show resolved Hide resolved
GitHubBranches.ps1 Outdated Show resolved Hide resolved
GitHubBranches.ps1 Outdated Show resolved Hide resolved
Tests/GitHubBranches.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubBranches.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubBranches.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubBranches.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubBranches.tests.ps1 Outdated Show resolved Hide resolved
@HowardWolosky HowardWolosky added the waiting for update Waiting for an update to the PR before the next review label Jul 1, 2020
Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Just a couple things I missed before.

GitHubBranches.ps1 Outdated Show resolved Hide resolved
GitHubBranches.ps1 Show resolved Hide resolved
GitHubBranches.ps1 Outdated Show resolved Hide resolved
GitHubBranches.ps1 Show resolved Hide resolved
Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for this!

@HowardWolosky
Copy link
Member

/azp run PowerShellForGitHub-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@HowardWolosky HowardWolosky removed the waiting for update Waiting for an update to the PR before the next review label Jul 16, 2020
@HowardWolosky HowardWolosky merged commit d76f54b into microsoft:master Jul 16, 2020
@X-Guardian X-Guardian deleted the GitHubBranches-NewRemove branch April 19, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api completeness This is basic API functionality that hasn't been implemented yet. api-refs Work to complete the API's defined here: https://developer.github.com/v3/git/refs/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants