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

Implement new gometalinter capabilities and take care of warnings about them #464

Merged
merged 12 commits into from
Sep 18, 2017

Conversation

rfay
Copy link
Member

@rfay rfay commented Sep 16, 2017

The Problem/Issue/Bug:

In drud/build-tools#42 @tannerjfco improved our handling of gometalinter quite a lot, and we should use that.

How this PR Solves The Problem:

  • Update to build-tools 1.5.1 which has the new gometalinter capabilities
  • Handle the warnings it now offers.
  • Implement vetshadow which warns about shadowed variables
  • gometalinter uses gofmt -s which insists on some simplifications in addition to the normal gofmt behavior. These can of course be calmed with a simple comment.

Key new issues:

  1. I changed all imports of testify/assert to be aliased as asrt to solve the shadowing problem of assert vs the import.
  2. Change imports of go-homedir from "homedir" alias to "gohomedir" alias for same reason.
  3. Since we're now using gometalinter instead of the individual tools, it's easy to use comment directives to calm down a particular tool if it's hitting a false positive (or even something you just don't want to deal with, like removing "dead" code that you don't consider dead.)

Manual Testing Instructions:

You may want to look at the changes commit-by-commit so there's not just a whole pile of changes.

Automated Testing Overview:

Related Issue Link(s):

drud/build-tools#42 was the build-tools upgrade for gometalinter.

Release/Deployment notes:

@rfay rfay self-assigned this Sep 16, 2017
@rfay rfay merged commit 22498e2 into ddev:master Sep 18, 2017
@rfay rfay deleted the 20170916_build_tools_with_gometalinter branch September 18, 2017 16:34
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