Skip to content

Fix linting#2903

Merged
gmlewis merged 5 commits intogoogle:masterfrom
WillAbides:lint
Sep 8, 2023
Merged

Fix linting#2903
gmlewis merged 5 commits intogoogle:masterfrom
WillAbides:lint

Conversation

@WillAbides
Copy link
Copy Markdown
Contributor

@WillAbides WillAbides commented Aug 26, 2023 •
edited
Loading

I noticed that .golangci.yml had some commented out and disabled linters. This PR fixes that. It also uncomments "example" from the GitHub action.

Running go mod tidy -compat 1.17 fixed the "no go files to analyze" error linting "examples".1

For the linting issues, I updated .golangci-lint and fixed the issues that it found.

I think most of the changes are obvious, but I want to mention a few things:

  • I created two test helpers "assertNilError" and "assertWrite" because that made the error checks issues easier to fix with find/replace. I see that assert functions are generally not used in this repo. I'm happy to inline these, but my preference is to keep them as-is.
  • I changed the signature of stringifyValue to accept a *bytes.Buffer instead of an io.Writer because the linter knows that Buffer.Write always returns nil errors.
  • I removed quite a bit of dead code from authorizations_test.go

Footnotes

  1. examples/go.* changes were originally part of this PR, but merging Remove dependency on "golang.org/x/oauth2" #2895 removed them from the diff. ↩

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 27, 2023 •
edited
Loading

Codecov Report

Merging #2903 (1f47817) into master (66b1147) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2903      +/-   ##
==========================================
+ Coverage   98.14%   98.17%   +0.03%     
==========================================
  Files         143      143              
  Lines       12606    12609       +3     
==========================================
+ Hits        12372    12379       +7     
+ Misses        159      156       -3     
+ Partials       75       74       -1
Files Changed Coverage Δ
github/actions_secrets.go 100.00% <100.00%> (ø)
github/github.go 98.11% <100.00%> (+<0.01%) ⬆️
github/repos_releases.go 91.13% <100.00%> (+1.68%) ⬆️
github/strings.go 100.00% <100.00%> (ø)

📢 Have feedback on the report? Share it here.

@WillAbides
Copy link
Copy Markdown
Contributor Author

WillAbides commented Aug 27, 2023

Codecov didn't like that I modified an uncovered line, so I added a test for it.

@gmlewis
Copy link
Copy Markdown
Collaborator

gmlewis commented Aug 27, 2023

I'm doing a crazy amount of driving today, so will be slow to respond. I try to "approve run" of the checks when I can, but today's going to be pretty challenging, sorry.

@WillAbides
Copy link
Copy Markdown
Contributor Author

WillAbides commented Aug 27, 2023

@gmlewis I appreciate how responsive you are on this repo. I have no expectation that you or anybody else be around on a weekend. I can see the lint results on my fork, so my feedback loop is just fine.

@WillAbides
Merge branch 'master' into lint
cf4a836 
# Conflicts:
#	example/go.mod
#	example/go.sum
gmlewis
gmlewis approved these changes Aug 28, 2023
View reviewed changes
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.

Wow, @WillAbides - thank you for this huge cleanup! It is greatly appreciated!
LGTM.

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

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Aug 28, 2023
gabriel-samfira
gabriel-samfira approved these changes Sep 8, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@gabriel-samfira gabriel-samfira left a comment

Choose a reason for hiding this comment

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

This is awesome

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 8, 2023
@gmlewis
Copy link
Copy Markdown
Collaborator

gmlewis commented Sep 8, 2023

Thank you, @gabriel-samfira !
Merging.

@gmlewis gmlewis merged commit fd33b81 into google:master Sep 8, 2023
@gmlewis gmlewis mentioned this pull request Sep 18, 2023
Merged
gmlewis pushed a commit to gmlewis/go-github that referenced this pull request Sep 19, 2023
@WillAbides @gmlewis
Fix linting (google#2903)
70f3172
@gmlewis gmlewis mentioned this pull request Oct 11, 2023
Merged
This was referenced Mar 17, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmlewis gmlewis gmlewis approved these changes

+1 more reviewer

@gabriel-samfira gabriel-samfira gabriel-samfira approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WillAbides @gmlewis @gabriel-samfira