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

Fix ListPullRequestsWithCommit option type #2822

Merged
merged 2 commits into from Jul 3, 2023

Conversation

RickleAndMortimer
Copy link
Contributor

@RickleAndMortimer RickleAndMortimer commented Jun 28, 2023

ListPullRequestsWithCommit uses PullRequestListOptions as one of it's parameters. This is incorrect because in GitHub's documentation, the endpoint for this function, repos/%v/%v/commits/%v/pulls, does not share the same parameters as the ones defined in ListPullRequestsWithCommit

go-github/github/pulls.go

Lines 119 to 141 in 96726d8

type PullRequestListOptions struct {
// State filters pull requests based on their state. Possible values are:
// open, closed, all. Default is "open".
State string `url:"state,omitempty"`
// Head filters pull requests by head user and branch name in the format of:
// "user:ref-name".
Head string `url:"head,omitempty"`
// Base filters pull requests by base branch name.
Base string `url:"base,omitempty"`
// Sort specifies how to sort pull requests. Possible values are: created,
// updated, popularity, long-running. Default is "created".
Sort string `url:"sort,omitempty"`
// Direction in which to sort pull requests. Possible values are: asc, desc.
// If Sort is "created" or not specified, Default is "desc", otherwise Default
// is "asc"
Direction string `url:"direction,omitempty"`
ListOptions
}

image

This PR simply sets the correct options, ListOptions, for the ListPullRequestWithCommit.

see #2815 for more details

Co-Authored-By: vgnshiyer <vgnshiyer@asu.edu>
@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Jun 28, 2023
@gmlewis
Copy link
Collaborator

gmlewis commented Jun 28, 2023

Thank you, @RickleAndMortimer !
Please run gofmt on the files you edited and push (not force push) the changes to this PR.
See CONTRIBUTING.md for more details.

@gmlewis
Copy link
Collaborator

gmlewis commented Jun 28, 2023

Fixes: #2815.

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #2822 (2dd9f5c) into master (96726d8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2822   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         136      136           
  Lines       12279    12279           
=======================================
  Hits        12041    12041           
  Misses        162      162           
  Partials       76       76           
Impacted Files Coverage Δ
github/pulls.go 96.77% <100.00%> (ø)

... and 1 file with indirect coverage changes

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, @RickleAndMortimer !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

Copy link
Contributor

@valbeat valbeat left a comment

Choose a reason for hiding this comment

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

LGTM!

@gmlewis gmlewis changed the title ListPullRequestsWithCommit has incorrect option type Fix ListPullRequestsWithCommit option type Jul 3, 2023
@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Jul 3, 2023
@gmlewis
Copy link
Collaborator

gmlewis commented Jul 3, 2023

Thank you, @valbeat !
Merging.

@gmlewis gmlewis merged commit 9f7124c into google:master Jul 3, 2023
9 checks passed
@RickleAndMortimer RickleAndMortimer deleted the list_pr_options_bugfix branch July 10, 2023 00:35
suzuki-shunsuke added a commit to suzuki-shunsuke/tfcmt that referenced this pull request Oct 7, 2023
suzuki-shunsuke added a commit to suzuki-shunsuke/tfcmt that referenced this pull request Oct 7, 2023
* fix(deps): update module github.com/google/go-github/v53 to v55

* fix: address API breaking changes

- google/go-github#2822

* fix: stop using deprecated API github.NewEnterpriseClient

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants