Skip to content

Fix ListPullRequestsWithCommit option type - #2822

Merged
gmlewis merged 2 commits into
google:masterfrom
RickleAndMortimer:list_pr_options_bugfix
Jul 3, 2023
Merged

Fix ListPullRequestsWithCommit option type#2822
gmlewis merged 2 commits into
google:masterfrom
RickleAndMortimer:list_pr_options_bugfix

Conversation

@RickleAndMortimer

@RickleAndMortimerRickleAndMortimer commented Jun 28, 2023

Copy link
Copy Markdown
Contributor

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

typePullRequestListOptionsstruct {
// State filters pull requests based on their state. Possible values are:
// open, closed, all. Default is "open".
Statestring`url:"state,omitempty"`
// Head filters pull requests by head user and branch name in the format of:
// "user:ref-name".
Headstring`url:"head,omitempty"`
// Base filters pull requests by base branch name.
Basestring`url:"base,omitempty"`
// Sort specifies how to sort pull requests. Possible values are: created,
// updated, popularity, long-running. Default is "created".
Sortstring`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"
Directionstring`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>
@gmlewisgmlewis 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
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

Fixes: #2815.

@codecov

codecovBot commented Jun 28, 2023

Copy link
Copy Markdown

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 FilesCoverage Δ
github/pulls.go96.77% <100.00%> (ø)

... and 1 file with indirect coverage changes

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

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.

@valbeatvalbeat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@gmlewisgmlewis changed the title ListPullRequestsWithCommit has incorrect option typeFix ListPullRequestsWithCommit option typeJul 3, 2023
@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Jul 3, 2023
@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @valbeat !
Merging.

@gmlewis
gmlewis merged commit 9f7124c into google:masterJul 3, 2023
@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>
evanelias added a commit to evanelias/go-github that referenced this pull request Feb 5, 2025
The docs for ListPullRequestsWithCommit mention use of PullRequestListOptions,
but this method no longer accepts PullRequestListOptions per google#2815 and google#2822.
This commit updates the method doc comment to remove the outdated reference
to PullRequestListOptions, and adds some clarifications based on the GitHub
docs for the relevant API endpoint.
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API ChangePR 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.

3 participants

@RickleAndMortimer@gmlewis@valbeat