This repository was archived by the owner on Aug 2, 2021. It is now read-only.
Enable golangci-lint and fix reported errors#1661
Merged
janos merged 1 commit intogo-modulesfrom Aug 14, 2019
Merged
Conversation
nolash
reviewed
Aug 9, 2019
| for i, n := range snap.Nodes { | ||
| gotServices := n.Node.Config.Services | ||
| for i := 0; i < len(snap.Nodes); i++ { | ||
| gotServices := snap.Nodes[i].Node.Config.Services |
Member
Author
There was a problem hiding this comment.
Previous iteration is coping the mutex, new one does not.
| # minimal length of string constant, 3 by default | ||
| min-len: 3 | ||
| # minimal occurrences count to trigger, 3 by default | ||
| min-occurrences: 5 |
Contributor
There was a problem hiding this comment.
What exactly do these two choices mean?
Member
Author
There was a problem hiding this comment.
Comments are taken from the original documentation. The first one is the minimal length of the string that can be considered for a constant and the second one is the minimal number of that string occurrences in the package to be reported as constant definition required.
nolash
approved these changes
Aug 12, 2019
skylenet
approved these changes
Aug 12, 2019
Member
Author
|
Thank you for reviews, I will merge this PR as soon as #1532 receives more approvals an is ready to be merged. |
holisticode
approved these changes
Aug 14, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fulfills go-modules PR by enabling linters that are used on master but in updated version report errors on the codebase. While go-modules PR does not make changes to the swarm application, only tooling, this one does and for easier review it is split into a separate changeset.
In go-modules PR, a deprecated gometalinter.v2 is replaced by golangci-lint as the older one does not play well with modules.
Goangci-lint configuration is added mainly for goconst linter to be more permissive on min-occurrences, as some of the changes required with default configuration are not required by my opinion. Even some strings are repeated multiple times, it does not mean that their semantics is the same justifying additional constant. This is debatable and open for discussion.
Running the complete set of goglanci-lint linters produce a very high number of errors and I am suggesting to address some of them in the future and standardize linting.