fix various linting issues #1048
Conversation
thaJeztah
force-pushed
the
fix_linting
branch
3 times, most recently
from
f309a37 to
6b4ed95
Compare
thaJeztah
force-pushed
the
fix_linting
branch
from
6b4ed95 to
bc285dc
Compare
thaJeztah
force-pushed
the
fix_linting
branch
from
bc285dc to
4d569a8
Compare
thaJeztah
force-pushed
the
fix_linting
branch
from
4d569a8 to
aed143d
Compare
|
minor nits otherwise lgtm, also please squash to a single commit |
link_linux.go:2913:6: func `addBondSlaveAttrs` is unused (unused)
func addBondSlaveAttrs(bondSlave *BondSlave, linkInfo *nl.RtAttr) {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
neigh_linux.go:125:6: func `neighAdd` is unused (unused)
func neighAdd(neigh *Neigh, mode int) error {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
qdisc_linux.go:770:6: func `time2Ktime` is unused (unused)
func time2Ktime(time uint32) uint32 {
^
qdisc_linux.go:774:6: func `ktime2Time` is unused (unused)
func ktime2Time(ktime uint32) uint32 {
^
qdisc_linux.go:778:6: func `burst` is unused (unused)
func burst(rate uint64, buffer uint32) uint32 {
^
qdisc_linux.go:782:6: func `latency` is unused (unused)
func latency(rate uint64, limit, buffer uint32) float64 {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
devlink_linux.go:327:7: S1009: should omit nil check; len() for []byte is defined as zero (gosimple)
if rawData != nil && len(rawData) == 1 {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
addr_linux.go:159:16: composites: github.com/vishvananda/netlink/nl.IfaCacheInfo struct literal uses unkeyed fields (govet)
cachedata := nl.IfaCacheInfo{unix.IfaCacheinfo{
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
conntrack_linux.go:501:6: func `parseRaw32` is unused (unused)
func parseRaw32(r *bytes.Reader, v *uint32) {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
netlink_test.go:57:6: S1005: unnecessary assignment to the blank identifier (gosimple)
if found, _ := foundRequiredMods[name]; !found {
^
netlink_test.go:221:6: S1005: unnecessary assignment to the blank identifier (gosimple)
if found, _ := foundRequiredMods[name]; !found {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
conntrack_linux.go:639:19: ineffectual assignment to l (ineffassign)
if nested, t, l = parseNfAttrTL(reader); nested && t == nl.CTA_TUPLE_IP {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
devlink_linux.go:262:2: field `rawData` is unused (unused)
rawData []byte
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
…error
route_test.go:1023:8: ineffectual assignment to err (ineffassign)
link, err := setUpRoutesBench(b)
^
route_test.go:1046:8: ineffectual assignment to err (ineffassign)
link, err := setUpRoutesBench(b)
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
nl/tc_linux.go:1079:2: const `__TCA_FLOWER_MAX` is unused (unused)
__TCA_FLOWER_MAX
^
ioctl_linux.go:31:2: const `_ETH_SS_NTUPLE_FILTERS` is unused (unused)
_ETH_SS_NTUPLE_FILTERS
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
I rebased and removed the TODO, but didn't squash; if possible I'd rather keep the commits separate as I tried to group changes in logical commits, which can provide much more context when looking at "blame" view than a giant squashed commit without context for the specific change. Let me know if that's a problem. |
|
The guidelines are one commit per PR, else its separate PRs. all commits in this pr are covering the same area (lint fixes) so IMO its fine to squash. |
|
Hm, is that documented somewhere? I'm not sure I agree with that; yes, PR should be an atomic / related set of changes, but requiring that to be a single commit would enforce larger changes to either be a long range of stacked PRs or bad practice and a commit that's too large to review. Commits should be logically grouped (and each commit should build), but I don't agree that a PR should always have a single commit. A patch, although minimal, like this, gives me context on why the change was made. Whereas putting this change in a large commit with "anything linting related" won't give me context at all why these changes where made; From e738db401e88c01a5cef16b9de2288d4fa8e2b8b Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <[email protected]>
Date: Mon, 13 Jan 2025 12:59:04 +0100
Subject: [PATCH] rename var that collided with builtin
Signed-off-by: Sebastiaan van Stijn <[email protected]>
---
xfrm_policy_linux.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xfrm_policy_linux.go b/xfrm_policy_linux.go
index bf143a1b..e6497b0a 100644
--- a/xfrm_policy_linux.go
+++ b/xfrm_policy_linux.go
@@ -344,8 +344,8 @@ func parseXfrmPolicy(m []byte, family int) (*XfrmPolicy, error) {
for _, attr := range attrs {
switch attr.Attr.Type {
case nl.XFRMA_TMPL:
- max := len(attr.Value)
- for i := 0; i < max; i += nl.SizeofXfrmUserTmpl {
+ maxVal := len(attr.Value)
+ for i := 0; i < maxVal; i += nl.SizeofXfrmUserTmpl {
var resTmpl XfrmPolicyTmpl
tmpl := nl.DeserializeXfrmUserTmpl(attr.Value[i : i+nl.SizeofXfrmUserTmpl])
resTmpl.Dst = tmpl.XfrmId.Daddr.ToIP() |
ineed it isnt, and requesting the squash is a reoccurring theme here. trying to get a contributing guide in to reflect the current policy by the maintainers. |
More to come, but these were fairly trivial fixes.