major refactor #4
Conversation
| @@ -240,7 +240,7 @@ type LinkLayerDiscoveryInfo struct { | |||
| Unknown []LinkLayerDiscoveryValue // undecoded TLVs | |||
| } | |||
|
|
|||
| /// IEEE 802.1 TLV Subtypes | |||
| // / IEEE 802.1 TLV Subtypes | |||
There was a problem hiding this comment.
this is an automate output of go fmt. not really keen to change it since it'll get reverted back to this on save anyway. I'll take a look in the context of that particular line though.
layers/mldv1_test.go
Outdated
| @@ -51,7 +54,7 @@ func TestPacketMulticastListenerQueryMessageV1(t *testing.T) { | |||
| t.Error("Failed to decode packet:", p.ErrorLayer().Error()) | |||
| } | |||
| checkLayers(p, []gopacket.LayerType{LayerTypeEthernet, LayerTypeIPv6, LayerTypeIPv6HopByHop, LayerTypeICMPv6, LayerTypeMLDv1MulticastListenerQuery}, t) | |||
| // See https://github.com/google/gopacket/issues/517 | |||
| // See https://github.com/gopacket/gopacket/issues/517 | |||
There was a problem hiding this comment.
I don't think this reference works anymore. Continue to point to google/gopacket?
(same comment across all other files that reference this issue)
There was a problem hiding this comment.
ah yes this needs to be reverted back. can't believe my findall/replace in vscode had negative consequences 😮
layers/sflow.go
Outdated
| // +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ | ||
| // | TOS | | ||
| // +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ | ||
| // 0 15 31 |
There was a problem hiding this comment.
I think the things in this file should be indented to be monospace
There was a problem hiding this comment.
how do you mean? looks like it's gone from spaces to tabs which is a go recommended way?
There was a problem hiding this comment.
Oh, I think this was a different error than what I was thinking (and one not intruduced in this PR). I was thinking these were godoc comments, like this:
//<one space>SFlowIpv4Record foos the bar
//<two spaces>packet format
If it had been, then that would have made the packet format monospace in the godoc. However, the lack of the first line with a //<single space>godoc start invalidates that. As is, all of these will (continue to) render in poorly-formatted non-monospace and probably look really weird in the godoc. (confirmed: https://pkg.go.dev/github.com/google/gopacket@v1.1.19/layers#SFlowIpv4Record)
If you'd like to leave as is, tots fine. However, if you're in here and want things to look a little nicer, you could change
//<two spaces>packet format
to
//<one space><Struct Name>
//
//<tab>packet format
There was a problem hiding this comment.
aaaah I got what you mean now. Happy to include it in the current PR
planning to merge this into master with references to original repo changed to gopacket/gopacket, and a 1.19 version of
go fmtapplied to the entire codebase. Will give this a couple of days so people can submit their PRs before so I don't have to deal with merge conflicts.