Repository navigation
Repositories - Merging #84
Description
Activity
Here are my proposals for this cmdlet:
Name: Merge-GitHubRepositoryBranches
FileName: GitHubRepositoryMerge.ps1
It takes the following parameters- OwnerName
- RepositoryName
- Uri
- AccessToken
- NoStatus
- Base (base in the API)
- Head (head in the API)
- CommitMessage (commit_message in the API)
Checklists
- Test the cmdlet using my own access token
- Add tests testing for different combinations of parameters
I have read the contributing.md and will try to follow the same.
Howard Wolosky (@HowardWolosky) would like to know if there are anything additional I need to keep in mind before implementationThanks @raghav710! Overall, this looks great. Looking forward to having this functionality.
Some minor feedback here:
- Please keep
AccessTokenandNoStatusas the last parameters in the method, just for consistency with the rest of the module. - You may want to call
Invoke-GHRestMethodwith the-ExtendedResultparameter so that you can detect a201vs a204and be able to doWrite-Log -Level Warningin the event that it's a204explaining that there's nothing to merge.
- Please keep
Howard Wolosky (@HowardWolosky) thanks for the note on the
201vs204. Looking forward to getting this in :)Me too.
The test may be hard to write though because we don't yet have API support for creating new pull requests... Something to keep in mind.
Reacted by Raghav SHoward Wolosky (@HowardWolosky) I think I need the ability to create branches so I can add tests for this. Shall I concentrate on adding that and then come back to this?
Also, here is a tentative PR: #89
I've tested the code manually using my own access tokenThanks Raghav! I've given your tentative PR an initial code review. I do think that it would be best to only check in features that have corresponding tests that can help ensure that the module remains working as expected. If you're able to add the necessary, tested methods for creating branches so that you can add tests for this PR, that would be great.
I think you may run into some trouble writing tests for the refs API's too though, because per the documentation, you can't create new refs on empty repos...so, you'd have to make sure that it has content in it first. It's possible that using
-AutoInitto create it with an empty README will be sufficient to satisfy that requirement however.Reacted by Raghav S
Issue to track the implementation of the APIs for "merging" under the "Repository" section in #70
API reference: https://developer.github.com/v3/repos/merging/
I will update this issue with my proposals for the cmdlet names and parameters