Skip to content
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

update exhaustive to latest; use version in go.mod #1449

Merged
merged 11 commits into from
Oct 14, 2020
Merged

update exhaustive to latest; use version in go.mod #1449

merged 11 commits into from
Oct 14, 2020

Conversation

nishanths
Copy link
Contributor

@nishanths nishanths commented Oct 12, 2020

This updates the exhaustive linter, and updates the dependency in go.mod to use vX.Y.Z format.

go get github.com/nishanths/exhaustive@v0.1.0
go mod tidy

Mainly address issue #3.

@nishanths
Copy link
Contributor Author

Tests are failing. I'll update.

go.mod Outdated
@@ -37,7 +37,7 @@ require (
github.com/mitchellh/go-ps v1.0.0
github.com/moricho/tparallel v0.2.1
github.com/nakabonne/nestif v0.3.0
github.com/nishanths/exhaustive v0.0.0-20200811152831-6cf413ae40e0
github.com/nishanths/exhaustive v0.0.0-20201012211509-7221e29c71f4
Copy link
Member

Choose a reason for hiding this comment

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

as you are maintainer of exhaustive, can you release tag version vX.Y.Z ? so that going forward dependenbot will just update it automatically ?

Copy link
Contributor Author

@nishanths nishanths Oct 12, 2020

Choose a reason for hiding this comment

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

I am, I'll do that.

@nishanths nishanths marked this pull request as ready for review October 12, 2020 22:25
@nishanths
Copy link
Contributor Author

This should be ready to review.

@nishanths
Copy link
Contributor Author

Once this is merged, can I request a new release of golangci-lint? A user is affected by a false positive that is fixed by this change: nishanths/exhaustive#3

@@ -102,6 +102,8 @@ linters-settings:
# see https://github.com/kisielk/errcheck#excluding-functions for details
exclude: /path/to/file.txt
exhaustive:
# check switch statements in generated files also
check-generated: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new flag in exhaustive.

@nishanths nishanths changed the title update exhaustive to latest update exhaustive to latest; use version in go.mod Oct 12, 2020
.golangci.yml Outdated Show resolved Hide resolved
@@ -9,8 +9,6 @@ linters-settings:
- github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
dupl:
threshold: 100
exhaustive:
default-signifies-exhaustive: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing because this is just the default value. As @bombsimon said, no need to mention defaults in this file.

Copy link
Member

@bombsimon bombsimon left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@sayboras
Copy link
Member

Once this is merged, can I request a new release of golangci-lint? A user is affected by a false positive that is fixed by this change: nishanths/exhaustive#3

In my opinion, we are doing monthly realease, so can I wait till next week ?

@nishanths
Copy link
Contributor Author

In my opinion, we are doing monthly realease, so can I wait till next week ?

Yes, that works very well.

@bombsimon bombsimon merged commit 58234f0 into golangci:master Oct 14, 2020
@nishanths nishanths deleted the update-exhaustive branch October 14, 2020 13:57
@ldez ldez added the linter: update version Update version of linter label Dec 7, 2020
@ldez ldez added this to the v1.32 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter: update version Update version of linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants