Some *Response return values are not covered by tests

[alexandear]

A few exported methods return *Response as the second result, but this value is never exercised by tests.

Based on the latest commit, the following methods are affected:

github/pulls.go:362:123: (*PullRequestsService).Edit
github/repos_releases.go:456:153: (*RepositoriesService).UploadReleaseAsset
github/search.go:81:122: (*SearchService).Repositories
github/search.go:119:110: (*SearchService).Topicsfunc
github/search.go:156:112: (*SearchService).Commits
github/search.go:178:110: (*SearchService).Issues
github/search.go:200:108: (*SearchService).Users
github/search.go:255:106: (*SearchService).Code
github/search.go:292:124: (*SearchService).Labels

This was discovered using the unparam linter, which reports result 1 (*github.com/google/go-github/v82/github.Response) is never used:

Details

❯ ./script/lint.sh
linting .
github/pulls.go:362:123: (*PullRequestsService).Edit - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *PullRequestsService) Edit(ctx context.Context, owner, repo string, number int, pull *PullRequest) (*PullRequest, *Response, error) {
                                                                                                                          ^
github/repos_releases.go:456:153: (*RepositoriesService).UploadReleaseAsset - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, repo string, id int64, opts *UploadOptions, file *os.File) (*ReleaseAsset, *Response, error) {
                                                                                                                                                        ^
github/search.go:81:122: (*SearchService).Repositories - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Repositories(ctx context.Context, query string, opts *SearchOptions) (*RepositoriesSearchResult, *Response, error) {
                                                                                                                         ^
github/search.go:119:110: (*SearchService).Topics - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Topics(ctx context.Context, query string, opts *SearchOptions) (*TopicsSearchResult, *Response, error) {
                                                                                                             ^
github/search.go:156:112: (*SearchService).Commits - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Commits(ctx context.Context, query string, opts *SearchOptions) (*CommitsSearchResult, *Response, error) {
                                                                                                               ^
github/search.go:178:110: (*SearchService).Issues - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Issues(ctx context.Context, query string, opts *SearchOptions) (*IssuesSearchResult, *Response, error) {
                                                                                                             ^
github/search.go:200:108: (*SearchService).Users - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Users(ctx context.Context, query string, opts *SearchOptions) (*UsersSearchResult, *Response, error) {
                                                                                                           ^
github/search.go:255:106: (*SearchService).Code - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Code(ctx context.Context, query string, opts *SearchOptions) (*CodeSearchResult, *Response, error) {
                                                                                                         ^
github/search.go:292:124: (*SearchService).Labels - result 1 (*github.com/google/go-github/v82/github.Response) is never used (unparam)
func (s *SearchService) Labels(ctx context.Context, repoID int64, query string, opts *SearchOptions) (*LabelsSearchResult, *Response, error) {
                                                                                                                           ^
9 issues:
* unparam: 9
failed linting .
linting example
/Users/alexandear/src/github.com/google/go-github/example
0 issues.
linting scrape
/Users/alexandear/src/github.com/google/go-github/scrape
scrape/apps.go:139:61: (*Client).CreateApp - result 0 (*net/http.Response) is never used (unparam)
func (c *Client) CreateApp(m *AppManifest, orgName string) (*http.Response, error) {
                                                            ^
1 issues:
* unparam: 1
failed linting scrape
linting tools
/Users/alexandear/src/github.com/google/go-github/tools
0 issues.
linting tools/fmtpercentv
/Users/alexandear/src/github.com/google/go-github/tools/fmtpercentv
0 issues.
linting tools/sliceofpointers
/Users/alexandear/src/github.com/google/go-github/tools/sliceofpointers
0 issues.
linting tools/structfield
/Users/alexandear/src/github.com/google/go-github/tools/structfield
0 issues.

Suggested fix is to add tests using the existing helper testNewRequestAndDoFailure.

Proposed steps

1. Enable check-exported: true in linters.settings.unparam in .golangci.yml.
2. Run ./script/lint.sh - 9 issues are reported in the github package.
3. Fix the listed methods by adding tests that assert the returned *Response via testNewRequestAndDoFailure.
4. Fix remaining unparam issues in other packages.

[alexandear]

This is a "good first issue" for anyone.

[gmlewis]

As @alexandear said above, this would be a great PR for any new contributor to this repo or a new Go developer.
All contributions are greatly appreciated!

Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work.
Also feel free to perform a code review on any PR that has the label "NeedsReview" - we appreciate those contributions too!
We have a REVIEWERS file that you could add your username to if you are willing to be contacted when reviews are needed.

Please check out our CONTRIBUTING.md guide to get started. (In particular, please remember to go generate ./... and don't use force-push to your PRs.)

Thank you!

[ManavSharma142]

I would like to work on this one!

[gmlewis]

I would like to work on this one!

Thank you, @ManavSharma142! It's yours.

[ManavSharma142]

@gmlewis @alexandear
Can you please tell if I need to update golangci.yml as well or just the test functions ?
Also,
Should I add testNewRequestAndDoFailure for scrape/apps.go:139:61: (*Client).CreateApp? But the helper function testNewRequestAndDoFailure is not available inside scrape so I need to create one there.

[alexandear]

@gmlewis @alexandear
Can you please tell if I need to update golangci.yml as well or just the test functions ?

Yes, update .golangci.yml as well to prevent future regressions.

Also,
Should I add testNewRequestAndDoFailure for scrape/apps.go:139:61: (*Client).CreateApp? But the helper function testNewRequestAndDoFailure is not available inside scrape so I need to create one there.

You shouldn't use testNewRequestAndDoFailure in the scrape/apps.go. Just fix it in somehow another way.