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

Makefile: Replace Gometalinter to Golangci-lint #13405

Merged
merged 5 commits into from
Dec 9, 2019

Conversation

xiekeyi98
Copy link
Contributor

@xiekeyi98 xiekeyi98 commented Nov 12, 2019

What problem does this PR solve?

Because of Gometalinter DEPRECATED, we should change it to golangci-lint.


This change is Reviewable

This PR will close #13675

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Nov 12, 2019
@codecov
Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #13405 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13405   +/-   ##
===========================================
  Coverage   80.1718%   80.1718%           
===========================================
  Files           481        481           
  Lines        120586     120586           
===========================================
  Hits          96676      96676           
  Misses        16188      16188           
  Partials       7722       7722

--enable misspell \
--enable ineffassign \
check-static: tools/bin/golangci-lint
tools/bin/golangci-lint run -v --disable-all --deadline=3m \
Copy link
Member

Choose a reason for hiding this comment

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

why changed to 3m ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, golangci-lint is faster than gometalinter.
But it cannot support like --deadline 120s , therefore I changed it to --deadline=3m .
The reason for 3m rather than 2m , beacuse I think we should give more time for the new tool.
And changed it back when it stable enough.

ngaut
ngaut previously approved these changes Nov 13, 2019
Copy link
Member

@ngaut ngaut left a comment

Choose a reason for hiding this comment

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

LGTM

@ngaut ngaut added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 13, 2019
@@ -271,6 +268,8 @@ tools/bin/misspell:tools/check/go.mod
tools/bin/ineffassign:tools/check/go.mod
cd tools/check; \
$(GO) build -o ../bin/ineffassign github.com/gordonklaus/ineffassign
tools/bin/golangci-lint:
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b ./tools/bin v1.21.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the binary from this URL change? Is there any version control for tools?
If the make result depends on the tool's version, it may trouble us sometimes. @xiekeyi98

We use go.mod to control the version of other tools here:
https://github.com/pingcap/tidb/blob/master/tools/check/go.mod

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, version control for tool is at last ( v1.21.0 ).
What do you think about we should do to install it.? I use this tool: https://github.com/golangci/golangci-lint

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao
Copy link
Contributor

Please resolve conflicts @xiekeyi98

@xiekeyi98 xiekeyi98 requested review from a team as code owners December 7, 2019 12:35
@ghost ghost requested review from qw4990, XuHuaiyu, francis0407 and alivxxx and removed request for a team December 7, 2019 12:36
@tiancaiamao
Copy link
Contributor

/run-all-tests

@alivxxx alivxxx added the status/can-merge Indicates a PR has been approved by a committer. label Dec 9, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 9, 2019

/run-all-tests

@sre-bot sre-bot merged commit 795c630 into pingcap:master Dec 9, 2019
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace gometalinter with golangci-lint
5 participants