Netvfy agent cleanup

[si74]

Note this PR has three key components:

1. adding a makefile to ensure that we are gofmting, running go vet, static checking, etc - this is catching a lot of errors (I noticed this due to my vs code auto-linter extensions)
2. adding more instructions to the Makefile
3. making some minor agent/arp package fixes based on the Makefile
4. go vendoring all the dependencies

[si74]

Yup running staticcheck ./... returned an error message for cases of checking var == false. I would suggest downloading the tool to run the make commands locally (https://staticcheck.io/). On a personal note, this is also a pattern I almost never see in all the Go code I've written. Typically one just does a !varName for booleans.

On Sat, Jun 5, 2021, 22:45 Nick Bouliane @.***> wrote:

@.**** commented on this pull request.

In arp.go https://github.com/netvfy/go-netvfy-agent/pull/14#discussion_r646063445:

@@ -62,7 +61,7 @@ func (t *ArpTable) Add(IP string) error {

// Check if an entry already exist before adding one.

_, ok := t.ArpMap.Load(IP)

• if ok == false {

you mean staticcheck ? can you show me what it says 🤔 is there a reference what is idiomatic go ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/netvfy/go-netvfy-agent/pull/14#discussion_r646063445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG65C5GY2VY2C5GMK3CTPLTRLONHANCNFSM46E43NUQ .

[si74]

@nicboul, Idecided to do a bit more research into idiomatic Go documentation and there is this -> https://golang.org/doc/effective_go#blank

I noticed, however, that this doesn't contain things that I've come to write/use very frequently over the last few years. A few such examples:

1. struct type assertions for interfaces -> var _ TestInterface = (*MyType)(nil)
2. declaring a slices capacity up front whereas it still has the length 0 -> val := make([]int, 0, 5) is preferable when possible (otherwise the underlying array needs to be resized as the slice is appended to)
3. Typically use ok as a boolean variable name when checking to see if a map contains a key. val, ok := mapA["keyA"]

Some of these are more along the line of style whereas others are optimizations (that the compiler would not otherwise do) and ways to force compile-time checks vs. runtime checks.

So I'd say that the best approach might be to: 1 - Check what the effective Go style guide says 2 - Rely on govet, staticcheck, and gofmt 3 - Consider other idiomatic Goisms from experience

[si74]

@nicboul fell down a rabbithole but I did actually find some more links re: golang style (super helpful for me to take a look at too in the future - curious how much of this is already integrated into staticcheck):

1. Uber style guide: https://github.com/uber-go/guide/blob/master/style.md (Taking a quick look at it, it is quite similar to patterns I'm seeing at Fastly.)
2. Google style guide: https://github.com/golang/go/wiki/CodeReviewComments (Has some really good stuff too - also it's from the inventors of Go so can't be wrong really)