Skip to content

refactor: Enable perfsprint; fix appeared lint issues - #3324

Merged
gmlewis merged 2 commits into
google:masterfrom
alexandear-org:refactor/enable-perfsprint
Oct 11, 2024
Merged

refactor: Enable perfsprint; fix appeared lint issues#3324
gmlewis merged 2 commits into
google:masterfrom
alexandear-org:refactor/enable-perfsprint

Conversation

@alexandear

@alexandearalexandear commented Oct 11, 2024

Copy link
Copy Markdown
Contributor

The PR enables perfsprint linter and fixes the lint issues that appeared. This linter is mostly useful for suggesting errors.New instead of fmt.Errorf where formatting is not needed.

Details
❯ golangci-lint run
github/copilot.go:90:11: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("assignee type field is not set")
^
github/git_commits.go:132:20: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, nil, fmt.Errorf("commit must be provided")
^
github/orgs_properties.go:72:12: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("non-string value in string array")
^
github/pulls.go:355:20: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, nil, fmt.Errorf("pull must be provided")
^
github/git_commits_test.go:405:30: fmt.Errorf can be replaced with errors.New (perfsprint)
signer := mockSigner(t, "", fmt.Errorf("signer error"), "")
^

@alexandear
alexandearforce-pushed the refactor/enable-perfsprint branch from 987bc82 to c047140CompareOctober 11, 2024 11:46
@alexandear
alexandearforce-pushed the refactor/enable-perfsprint branch from c047140 to 58ba564CompareOctober 11, 2024 11:47

@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.

I'm fine with fmt.Errorf => errors.New but am not OK with fmt.Sprint => strconv.FormatInt.

Comment threadgithub/dependabot_secrets.go Outdated
Comment threadgithub/github_test.go Outdated
@codecov

codecovBot commented Oct 11, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 93.29%. Comparing base (2b8c7fa) to head (3d9f8e2).
Report is 146 commits behind head on master.

Files with missing linesPatch %Lines
...xample/codespaces/newreposecretwithxcrypto/main.go0.00%1 Missing ⚠️
...xample/codespaces/newusersecretwithxcrypto/main.go0.00%1 Missing ⚠️
example/newreposecretwithxcrypto/main.go0.00%1 Missing ⚠️
github/pulls.go0.00%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3324 +/- ##
==========================================
- Coverage 97.72% 93.29% -4.43% 
==========================================
Files 153 171 +18 Lines 13390 11729 -1661 ==========================================
- Hits 13085 10943 -2142 - Misses 215 692 +477 - Partials 90 94 +4 

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

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

Sign up for freeto 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.

2 participants

@alexandear@gmlewis