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

Fix linter non-style issues, add modules #42

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

nezorflame
Copy link
Contributor

@nezorflame nezorflame commented Nov 28, 2018

This PR fixes few issues reported by golangci-lint linters golint, govet, gosec, unused and errcheck.
Also adds preliminary support for Go modules.

@nezorflame nezorflame mentioned this pull request Nov 28, 2018
Copy link
Owner

@xeals xeals left a comment

Choose a reason for hiding this comment

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

Mostly okay, couple of changes or clarifications I'd like.

What linter is this? golint isn't giving these unused result warnings for me. Nothing I've used ever suggested the change similar to that made in types/util.go's rescue function either.

types/backup.go Outdated Show resolved Hide resolved
types/backup.go Outdated Show resolved Hide resolved
@nezorflame
Copy link
Contributor Author

nezorflame commented Nov 28, 2018

I stand corrected - these warnings were produced by golangci-lint, which uses multiple linters internally, including golint.
These warnings were given by it.

Most of the changes are fixes for the warning of the errcheck linter, which checks for the missing error checks in the code.
The warning given was Error return value of ... is not checked (errcheck)

@nezorflame nezorflame changed the title Fix golint non-style issues Fix linter non-style issues Nov 28, 2018
@nezorflame
Copy link
Contributor Author

There are still some warnings left, but they are mostly non-essential.
If you'd like, I can post you the full output here.

@nezorflame
Copy link
Contributor Author

nezorflame commented May 30, 2019

Ready to be reviewed again.
Also I've added initial Go modules support since dependency management is pretty stable these days (we use it at my work in production without any issues).
This will also allow people to use go get again for this binary starting with Go 1.12, because thanks to the modules, it'll use correct dependency versions.

@nezorflame nezorflame changed the title Fix linter non-style issues Fix linter non-style issues, add modules May 30, 2019
@m0
Copy link

m0 commented Jul 2, 2019

Thanks for adding the modules @nezorflame! You might want to split the module patch and the style changes into two separate PRs for easier review. While I have no opinion about the style changes, I think the added modules are a great improvement.

@nezorflame
Copy link
Contributor Author

@m0 @xeals as requested, I've separated style changes with the module PR (see #61).

So far it seems that all the essential golangci-lint issues have been addressed.

@nezorflame nezorflame requested a review from xeals August 4, 2020 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants