refactor: enable linter rule ST1016 for 'consistent method receiver names' #86

mwbrooks · 2025-05-09T16:29:01Z

Summary

This pull request enables the linter staticcheck rule ST1016: 'Use consistent method receiver names'.

For each, I choose the receiver name that was used the majority of the time.

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

codecov · 2025-05-09T16:31:36Z

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 63.19%. Comparing base (53f1124) to head (8212d45).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/prompts/app_select.go	75.00%	3 Missing ⚠️
internal/slackdeps/os.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   63.17%   63.19%   +0.01%     
==========================================
  Files         210      210              
  Lines       22186    22186              
==========================================
+ Hits        14017    14020       +3     
+ Misses       7082     7081       -1     
+ Partials     1087     1085       -2

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

mwbrooks · 2025-05-09T16:30:43Z

internal/prompts/app_select.go

 // auth or an app that corresponds. E.g. when you are comparing to --team flags
-func (t *TeamApps) authOrAppTeamDomain() string {
-	if t.Auth.TeamDomain != "" && (t.Auth.TeamID == t.Hosted.App.TeamID || t.Auth.TeamID == t.Local.App.TeamID) {
+func (apps *TeamApps) authOrAppTeamDomain() string {


note: The apps receiver name is used the majority of the time. However, I noticed that Golang encourages short, 1-or-2 letter receiver names. In this case a, t, or ta would be more appropriate. However, this file is quite long and apps reads well. So I leaned toward less changes.

In implementations having a more complete variable name seems better to me. Thanks for linking these docs though. It makes for interesting considerations... 🎁

The name need not be as descriptive as that of a method argument, as its role is obvious and serves no documentary purpose.

Sometimes I find it a hassle to search for single letter variables FWIW but this is the most interesting consideration. Perhaps I am needing to read more go 🙏 ✨

Edit: Wow I find the change that follows this in os.go makes sense as a single letter. So much to consider.

Yea, I'm with you. Sometimes the single letter just doesn't feel right. For TeamApps, maybe rethinking the struct's name (or purpose) will help? But, overall I'm cool with apps :)

zimeg

@mwbrooks Once more a wonderful PR to improve the health of code 🏥 ✨

These are appreciated and it's motivating to find a few of these landing so often - I'm feeling better and better about the code and hadn't even realized these linting rules were broken before the latest golangci-lint updates 🔍

zimeg · 2025-05-13T06:20:55Z

.golangci.yml

        - '-QF1004' # disable rule 'Use strings.ReplaceAll instead of strings.Replace'
        - '-QF1008' # disable rule 'Omit embedded fields from selector'
        - '-QF1011' # disable rule 'Omit redundant type from variable declaration'
-        - '-ST1016' # disable rule 'Use consistent method receiver names'


zimeg · 2025-05-13T06:22:00Z

internal/iostreams/iostreams_mock.go

 // SetExitCode sets the desired exit code in a thread-safe way
-func (io *IOStreamsMock) SetExitCode(code ExitCode) {
-	atomic.StoreInt32((*int32)(&io.exitCode), int32(code))
+func (m *IOStreamsMock) SetExitCode(code ExitCode) {


🧪 Nice! I like mock being m if this is what's found elsewhere!

Yea, m seems to be a common receiver name for mocks. Makes sense to me and keeps it consistent.

zimeg · 2025-05-13T06:22:47Z

internal/prompts/app_select.go

 // auth or an app that corresponds. E.g. when you are comparing to --team flags
-func (t *TeamApps) authOrAppTeamDomain() string {
-	if t.Auth.TeamDomain != "" && (t.Auth.TeamID == t.Hosted.App.TeamID || t.Auth.TeamID == t.Local.App.TeamID) {
+func (apps *TeamApps) authOrAppTeamDomain() string {


In implementations having a more complete variable name seems better to me. Thanks for linking these docs though. It makes for interesting considerations... 🎁

The name need not be as descriptive as that of a method argument, as its role is obvious and serves no documentary purpose.

Sometimes I find it a hassle to search for single letter variables FWIW but this is the most interesting consideration. Perhaps I am needing to read more go 🙏 ✨

zimeg · 2025-05-13T06:27:43Z

internal/prompts/app_select.go

 // auth or an app that corresponds. E.g. when you are comparing to --team flags
-func (t *TeamApps) authOrAppTeamDomain() string {
-	if t.Auth.TeamDomain != "" && (t.Auth.TeamID == t.Hosted.App.TeamID || t.Auth.TeamID == t.Local.App.TeamID) {
+func (apps *TeamApps) authOrAppTeamDomain() string {


Edit: Wow I find the change that follows this in os.go makes sense as a single letter. So much to consider.

mwbrooks · 2025-05-13T17:01:10Z

Thanks for the review @zimeg! While we don't have to take every linting rule to heart, it definitely helps to keep the code more consistent and readable. For the most part, I think we're already following the short receiver names, so I don't think we'll have trouble keeping with it! 🚀

mwbrooks added 2 commits April 28, 2025 13:40

refactor: enable linter ST1016 'Use consistent method receiver names'

0a86e41

Merge branch 'main' into mwbrooks-lint-st1016

461180d

mwbrooks added this to the Next Release milestone May 9, 2025

mwbrooks self-assigned this May 9, 2025

mwbrooks added code health M-T: Test improvements and anything that improves code health semver:patch Use on pull requests to describe the release version increment labels May 9, 2025

mwbrooks commented May 9, 2025

View reviewed changes

mwbrooks marked this pull request as ready for review May 9, 2025 16:33

mwbrooks requested a review from a team as a code owner May 9, 2025 16:33

Merge branch 'main' into mwbrooks-lint-st1016

8212d45

zimeg approved these changes May 13, 2025

View reviewed changes

mwbrooks merged commit dede31d into main May 13, 2025
6 checks passed

mwbrooks deleted the mwbrooks-lint-st1016 branch May 13, 2025 17:01

refactor: enable linter rule ST1016 for 'consistent method receiver names' #86

refactor: enable linter rule ST1016 for 'consistent method receiver names' #86

Uh oh!

Conversation

mwbrooks commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Requirements

Uh oh!

codecov bot commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwbrooks commented May 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mwbrooks commented May 9, 2025 •

edited

Loading

codecov bot commented May 9, 2025 •

edited

Loading