Skip to content

refactor: Remove redundant github.Ptr calls#4145

Merged
gmlewis merged 2 commits intogoogle:masterfrom
alexandear-org:refactor/redundantptr
Apr 10, 2026
Merged

refactor: Remove redundant github.Ptr calls#4145
gmlewis merged 2 commits intogoogle:masterfrom
alexandear-org:refactor/redundantptr

Conversation

@alexandear
Copy link
Copy Markdown
Contributor

This PR adds redundantptr linter to detect github.Ptr(x) calls that can be replaced with simple &x.

@gmlewis gmlewis changed the title refactor: Remove redundant github.Ptr calls refactor: Remove redundant github.Ptr calls Apr 9, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.70%. Comparing base (5124fac) to head (28ea325).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
example/commitpr/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4145      +/-   ##
==========================================
- Coverage   93.74%   93.70%   -0.04%     
==========================================
  Files         211      210       -1     
  Lines       19685    18996     -689     
==========================================
- Hits        18453    17801     -652     
+ Misses       1034     1009      -25     
+ Partials      198      186      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Apr 10, 2026
@alexandear alexandear requested a review from gmlewis April 10, 2026 06:48
Copy link
Copy Markdown
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, @alexandear!
Just one file deletion, then LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @zyfy29 - @Not-Dhananjay-Mishra - @munlicode

@@ -0,0 +1,10 @@
// Copyright 2026 The go-github AUTHORS. All rights reserved.
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.

I think this file can be completely deleted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't remove this file because it's used in tests:

go test ./...
--- FAIL: TestRun (0.16s)
    analysistest.go:417: src/has-warnings/main.go:9:2: cannot find package "github.com/google/go-github/v84/github" in any of:
                /opt/homebrew/Cellar/go/1.26.2/libexec/src/github.com/google/go-github/v84/github (from $GOROOT)
                /Users/alexandear/src/github.com/google/go-github/tools/redundantptr/testdata/src/github.com/google/go-github/v84/github (from $GOPATH)
    analysistest.go:417: /Users/alexandear/src/github.com/google/go-github/tools/redundantptr/testdata/src/has-warnings/main.go:9:2: could not import github.com/google/go-github/v84/github (invalid package name: "")
    analysistest.go:417: /Users/alexandear/src/github.com/google/go-github/tools/redundantptr/testdata/src/no-warnings/main.go:8:8: could not import github.com/google/go-github/v84/github (invalid package name: "")
    analysistest.go:431: error analyzing redundantptr@has-warnings: analysis skipped due to errors in package
    analysistest.go:431: error analyzing redundantptr@no-warnings: analysis skipped due to errors in package
FAIL
FAIL    tools/redundantptr      0.379s
FAIL

The analysistest framework needs the imported packages to be available in the testdata directory structure.

@alexandear alexandear requested a review from gmlewis April 10, 2026 12:16
Copy link
Copy Markdown
Contributor

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +5 to +13
require (
github.com/golangci/plugin-module-register v0.1.2
golang.org/x/tools v0.43.0
)

require (
golang.org/x/mod v0.34.0 // indirect
golang.org/x/sync v0.20.0 // indirect
)
Copy link
Copy Markdown
Collaborator

@gmlewis gmlewis Apr 10, 2026

Choose a reason for hiding this comment

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

Would this work?
I had to do something similar in tools/check-structfield-settings/go.mod.

require (
	github.com/golangci/plugin-module-register v0.1.2
	github.com/google/go-github/v84/github v0.0.0
	golang.org/x/tools v0.43.0
)

require (
	golang.org/x/mod v0.34.0 // indirect
	golang.org/x/sync v0.20.0 // indirect
)

// Use version at HEAD, not the latest published.
replace github.com/google/go-github/v84/github v0.0.0 => ../../../github

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.

WHUPS! See edit above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, it won't work. analysistest uses GOPATH-style package resolution under testdata/src/ and doesn't consult go.mod or replace directives at all.

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.

Bummer, OK.

Copy link
Copy Markdown
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, @alexandear and @Not-Dhananjay-Mishra!
LGTM.
Merging.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Apr 10, 2026
@gmlewis gmlewis merged commit 7d06267 into google:master Apr 10, 2026
7 of 8 checks passed
@alexandear alexandear deleted the refactor/redundantptr branch April 10, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants