decred / vspd

A Voting Service Provider (VSP) for the Decred network.
ISC License
19 stars 20 forks source link

Use go 1.22 language features. #488

Open jholdstock opened 1 month ago

jholdstock commented 1 month ago

Based on #487

jholdstock commented 1 month ago

Thats why I opened this as a separate PR, I am still in two minds about it.

IMO updating the for loops is worthwhile because it tidies up the code and removes the chance of off-by-one errors (and other bugs), but I'm not sure it merits a version bump on its own. Perhaps this PR can remain open as a TODO, to be revisited on the next go version bump.

davecgh commented 1 month ago

IMO updating the for loops is worthwhile because it tidies up the code and removes the chance of off-by-one errors (and other bugs), but I'm not sure it merits a version bump on its own.

I guess I'm just too used to normal for loops, but I find them more obvious and less error prone than the new syntax myself.

When I see a range, it requires thinking about whether it's the index or the value (like in maps) which means looking at the type of the target of the range and remembering which inconsistent semantics are at play. Then, there is the whole change so that the loop variable is per-iteration scope instead of per-loop scope in Go 1.22 which means the go.mod version needs to be consulted when reviewing to know which behavior you're dealing with. Now, you also have the pleasure of consulting the go.mod version yet again to determine if the target can even be ranged over (for the new types).

On the other hand, for i := 0; i < num; i++ { is 100% obvious exactly what it's going to do with no additional context.

jholdstock commented 1 week ago
==> lint main module
WARN [linters_context] copyloopvar: this linter is disabled because the Go version (1.20) of your project is lower than Go 1.22 
==> lint client
WARN [linters_context] copyloopvar: this linter is disabled because the Go version (1.19) of your project is lower than Go 1.22 
==> lint types
WARN [linters_context] copyloopvar: this linter is disabled because the Go version (1.19) of your project is lower than Go 1.22 

Newly introduced copyloopvar linter does not run unless go.mod files are updated.