Skip to content

Conversation

@shotat
Copy link
Contributor

@shotat shotat commented Sep 4, 2019

Add support for BranchListOptions for RepositoriesService.ListBranches.

ref. https://developer.github.com/v3/repos/branches/#list-branches

@googlebot googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Sep 4, 2019
@shotat
Copy link
Contributor Author

shotat commented Sep 4, 2019

This branch failed to pass CI because of Go 1.13.
Could anyone fix the CI settings?

@shotat shotat mentioned this pull request Sep 4, 2019
@codecov
Copy link

codecov bot commented Sep 7, 2019

Codecov Report

Merging #1272 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1272   +/-   ##
=======================================
  Coverage   73.42%   73.42%           
=======================================
  Files          86       86           
  Lines        6040     6040           
=======================================
  Hits         4435     4435           
  Misses        836      836           
  Partials      769      769
Impacted Files Coverage Δ
github/repos.go 75.16% <100%> (ø) ⬆️
github/checks.go 62.68% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e90302...7f7e23d. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @shotat, and sorry for the delay.
Could you please make the minor comment change I'm requesting then we will merge this after we get a second LGTM.

github/repos.go Outdated
// Setting to true returns only protected branches.
// When set to false, only unprotected branches are returned.
// Omitting this parameter returns all branches.
// Default: null
Copy link
Collaborator

Choose a reason for hiding this comment

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

In Go, we prefer to say nil over null. Could you please change this?

@gmlewis gmlewis requested a review from gauntface September 7, 2019 20:22
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @shotat!
LGTM.

Awaiting second LGTM before merging.

@shotat
Copy link
Contributor Author

shotat commented Sep 8, 2019

Thanks for your review!

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 10, 2019

Thank you, @gauntface!
Merging.

@gmlewis gmlewis merged commit e389f86 into google:master Sep 10, 2019
n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indication that the PR author has signed a Google Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants