Skip to content

dev: enable revive linter with default config #3622

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 1 commit into from
Apr 2, 2023

Conversation

alexandear
Copy link
Member

@alexandear alexandear commented Feb 21, 2023

This PR enables revive in the .golangci.yml config.

@alexandear alexandear marked this pull request as draft February 21, 2023 17:38
@ldez ldez self-requested a review February 21, 2023 18:10
@ldez
Copy link
Member

ldez commented Feb 21, 2023

All the linters are always not-used for the golangci-lint repository when they are introduced, it's expected.

As we run on several versions of golangci-lint in the CI, a problem exists when a linter doesn't exist yet.
Also, the changes in linters can lead to an unresolvable situation for the same reason.

So, we have to take care of enabling a linter for the golangci-lint repository.

@ldez ldez added the enhancement New feature or improvement label Feb 21, 2023
@ldez
Copy link
Member

ldez commented Feb 21, 2023

I don't understand why this PR is in draft, can you explain it?

@ldez ldez added the feedback required Requires additional feedback label Feb 21, 2023
@alexandear alexandear marked this pull request as ready for review February 21, 2023 18:59
@alexandear
Copy link
Member Author

I need to add manually revive rules because they are disabled by default. That's why drafted PR.

@alexandear alexandear removed the feedback required Requires additional feedback label Feb 21, 2023
@ldez
Copy link
Member

ldez commented Feb 21, 2023

I need to add manually revive rules because they are disabled by default. That's why drafted PR.

revive rules are not disabled by default: there is a default rule set.

In this context, it's better to prepare your branch locally before opening a PR.
Each PR and each commit send notifications.

ldez
ldez previously requested changes Feb 21, 2023
@alexandear alexandear requested a review from ldez February 21, 2023 20:03
@alexandear alexandear dismissed ldez’s stale review February 24, 2023 20:24

I resolved all comments. Looks like it's a GitHub's bug.

@ldez
Copy link
Member

ldez commented Feb 27, 2023

Screenshot 2023-02-27 at 22-13-26 dev enable revive linter by alexandear · Pull Request #3622 · golangci_golangci-lint

Never dismiss a review and especially mine.

When you resolve comments, you can ask for another review but not dismiss a review, I will remove the dismiss right.

@alexandear alexandear changed the title dev: enable revive linter dev: enable revive linter with default config Mar 17, 2023
@ldez ldez merged commit 5d34936 into golangci:master Apr 2, 2023
@alexandear alexandear deleted the enable-revive branch April 2, 2023 15:20
@thejan2009
Copy link

Hey, revive config works different than expected. With this change the linter was enabled, but its rule set was reduced to just a disabled unexported-return. After removing revive: from linters-config:, we get the following:

$ ./golangci-lint run --max-same-issues 1
pkg/golinters/gocritic_test.go:59:22: unused-parameter: parameter 'name' seems to be unused, consider removing or renaming it as _ (revive)
func (l *tLog) Child(name string) logutils.Log { return nil }
                     ^
pkg/golinters/gocritic_test.go:61:25: unused-parameter: parameter 'level' seems to be unused, consider removing or renaming it as _ (revive)
func (l *tLog) SetLevel(level logutils.LogLevel) {}
                        ^
pkg/printers/checkstyle.go:44:27: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
func (p Checkstyle) Print(ctx context.Context, issues []result.Issue) error {
                          ^
pkg/printers/junitxml.go:85:2: if-return: redundant if ...; err != nil check, just return error instead. (revive)
        if err := enc.Encode(res); err != nil {
                return err
        }
pkg/logutils/logutils.go:82:16: unused-parameter: parameter 'format' seems to be unused, consider removing or renaming it as _ (revive)
func nopDebugf(format string, args ...any) {}
               ^
pkg/result/processors/sort_results.go:95:47: unexported-return: exported method Compare returns unexported type processors.compareResult, which can be annoying to use (revive)
func (cmp ByName) Compare(a, b *result.Issue) compareResult {
                                              ^

@alexandear
Copy link
Member Author

@thejan2009 thank you for pointing this out. I will create a PR to enable recommended rules.

SeigeC pushed a commit to SeigeC/golangci-lint that referenced this pull request Apr 4, 2023
@ldez ldez added this to the v1.53 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants