Remove unused code, add staticcheck to TravisCI #2465
Conversation
sparrc
force-pushed
the
static-analyser-fixes
branch
5 times, most recently
from
010ee89
to
5a39be7
Compare
sparrc
force-pushed
the
static-analyser-fixes
branch
from
5a39be7
to
503be9b
Compare
sparrc
changed the title
Makefile
Outdated
| @@ -263,6 +263,12 @@ gocyclo: | |||
| # Run gocyclo over all .go files | |||
| gocyclo -over 15 ${GOFILES} | |||
|
|
|||
| .PHONY: staticcheck | |||
There was a problem hiding this comment.
https://github.com/aws/amazon-ecs-agent/blob/master/Makefile#L330 we have static-check already, can we name this differently? unused-check?
There was a problem hiding this comment.
I prefer staticcheck because it's the name of the tool we're using (https://github.com/dominikh/go-tools). We can't use unused-check because staticcheck does other checks that we will be adding soon.
If you'd prefer maybe I could do go-tools-staticcheck ?
There was a problem hiding this comment.
I put it as a command under the static-check target
| func TestIsNetworkStatsError(t *testing.T) { | ||
| isNetStatsErr := isNetworkStatsError(fmt.Errorf("no such file or directory")) | ||
| if isNetStatsErr { | ||
| // Expect it to not be a net stats error | ||
| t.Error("Error incorrectly reported as network stats error") | ||
| } | ||
|
|
||
| isNetStatsErr = isNetworkStatsError(fmt.Errorf("open /sys/class/net/veth2f5f3e4/statistics/tx_bytes: no such file or directory")) | ||
| if !isNetStatsErr { | ||
| // Expect this to be a net stats error | ||
| t.Error("Error incorrectly reported as non network stats error") | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Why is this test removed?
There was a problem hiding this comment.
This was removed because the function it's testing was removed (isNetworkStatsError)
sparrc
force-pushed
the
static-analyser-fixes
branch
from
503be9b
to
68ebfc6
Compare
sparrc
force-pushed
the
static-analyser-fixes
branch
from
68ebfc6
to
2919a29
Compare
Summary
This PR mostly removes unused code found using the staticcheck tool (plus a couple other static analysis fixups). It also adds this tool to our static-check make target so that it runs on every PR in Travis.
There will be PRs following up that will fix more static analysis issues and adding these checks to CI.
Testing
New tests cover the changes:
staticcheck tool was added to our Travis CI static-check make target. Currently only running the unused code check (U1000).
Description for the changelog
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.