Skip to content

refactor: removes get from getters names #1373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 24, 2025
Merged

refactor: removes get from getters names #1373

merged 3 commits into from
May 24, 2025

Conversation

chavacava
Copy link
Collaborator

This PR removes the prefix get from getter methods (see Getters)

Note: this PR includes modifications in 3 functions from main that are not strictly getters but removing the get prefix makes them read better.

// GetPattern - returns the actual pattern
func (p *LintPattern) GetPattern() string {
// Pattern - returns the actual pattern
func (p *LintPattern) Pattern() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

This struct is exported, so you are updating the method name of something that could be used by someone.

And you don't provide a method with deprecated flag.

So I'm doubtful here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes you made

lint/failure.go Outdated
Comment on lines 84 to 86
func (f *Failure) GetFilename() string {
// Filename returns the filename.
func (f *Failure) Filename() string {
return f.Position.Start.Filename
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as

#1373 (comment)

lint/failure.go Outdated
Comment on lines 83 to 87
// GetFilename returns the filename.
// Deprecated: Use Filename.
func (f *Failure) GetFilename() string {
return f.Filename()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to respect specs, it should be in a separated paragraph.

Suggested change
// GetFilename returns the filename.
// Deprecated: Use Filename.
func (f *Failure) GetFilename() string {
return f.Filename()
}
// GetFilename returns the filename.
//
// Deprecated: Use Filename.
func (f *Failure) GetFilename() string {
return f.Filename()
}

Also, you could use godoc link to help navigation

Suggested change
// GetFilename returns the filename.
// Deprecated: Use Filename.
func (f *Failure) GetFilename() string {
return f.Filename()
}
// GetFilename returns the filename.
//
// Deprecated: Use [Filename].
func (f *Failure) GetFilename() string {
return f.Filename()
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can detect this with gocritic.

I create a separate PR #1375 to enable gocritic

Comment on lines 14 to 15
// GetPattern - returns the actual pattern
// Deprecated: Use Pattern.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// GetPattern - returns the actual pattern
// Deprecated: Use Pattern.
// GetPattern - returns the actual pattern.
//
// Deprecated: Use [Pattern].

// GetPattern - returns the actual pattern
func (p *LintPattern) GetPattern() string {
// Pattern - returns the actual pattern
func (p *LintPattern) Pattern() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes you made

Copy link
Collaborator

@alexandear alexandear left a comment

Choose a reason for hiding this comment

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

Please fix comments and can be merged.

Copy link
Collaborator

@denisvmedia denisvmedia left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@chavacava chavacava merged commit 94cd7bd into master May 24, 2025
8 checks passed
@chavacava chavacava deleted the no-getters branch May 26, 2025 11:19
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.

4 participants