-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
improve linter quality for external-dns #1618
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njuettner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@szuecs thanks for the suggestion of adding staticcheck. If something is missing, let me know. |
issues: | ||
# Excluding configuration per-path, per-linter, per-text and per-source | ||
exclude-rules: | ||
- path: _test\.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@njuettner do we really want to disable all checks for the test code? Usually I prefer to think about test code to be equally important to production one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should not, however I would rather start with this approach and then as soon as possible add the linter for test files. The only problem which I faced was it is simply impossible to fix it within 2 hours. There were so many complaints in the test files which was too much to handle.
I would like to merge it because I want to have a linter which also checks if code it not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, cool. I've created #1621 to handle it later
/lgtm |
Towards #1555
It seems me missed quite a lot of good linters. I added more and there were also a lot of minor nits inside the code which I've fixed.
The goal here would be to improve in general our code quality. It make sense for the future to even add more linters but I think this is a good starting point.