chore(lint): disable shadowing checks with govet

[qdm12]

Changes

• Disable shadowing check (disabled by default) with govet
• In my opinion:
  • these make the code longer since we need to pre-declare things
  • you can't have locally-scoped variables in an if or for block for example if the variable name is already used in the upper scope.

Feel free to comment & close it if you disagree with the change 😉

Tests

golangci-lint run

Issues

• No issue

Primary Reviewer

• No-one

[jimjbrettj]

I like it. When I first joined I was having this happen to me with "err" specifically but we wanted to have consistent error variable names (which makes sense), so I'm a fan of this change

[codecov]

Codecov Report

Merging #1940 (b0fc726) into development (becec9e) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           development    #1940   +/-   ##
============================================
  Coverage        60.26%   60.26%           
============================================
  Files              180      180           
  Lines            18189    18189           
============================================
  Hits             10962    10962           
  Misses            5415     5415           
  Partials          1812     1812           

Flag	Coverage Δ
unit-tests	60.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update becec9e...b0fc726. Read the comment docs.

[kishansagathiya]

I was having this happen to me with "err" specifically but we wanted to have consistent error variable names (which makes sense).

I think, generally, the shadowing check appears when you are doing something like k, err :=, where err is an old variable. (Is that right, or am I confusing with some other check)

Personally, it isn't a big pain for me to declare the variable separately to avoid this warning.

The check is, however, helpful, because it ensures that you are not using a different copy of some variable (err is getting re-initialized in above example). I have faced at least one bug because of this, and this check could prevent stuff like that.

you can't have locally-scoped variables in an if or for block for example if the variable name is already used in the upper scope.

Using a different variable name is less confusing for reader.

[noot]

I agree with Kishan, I don't see this check as bad, it's helped catch bugs in the past. sure it can be annoying, but I'd rather have to declare a variable separately than have some bug

[qdm12]

Using a different variable name is less confusing for reader.

Definitely. Although my problem is more about err. I'm not going to use a different error variable name for every error returned 😄 And that means I always have to add a line to define variables before a line returning an error, this is really slowing me down and adds code.

To illustrate

a, err := f()
// ...
var b int
b, err = g()
// above you cannot do b, err := g() 😢

EDIT: I'll check you might be able to disable it for errors using the exclude rules

[qdm12]

Shadowing detection is re-enabled, but shadow detection of errors (err) is disabled.

[github-actions]

🎉 This PR is included in version 0.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀