search

lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
262 stars 125 forks source link

Require Go 1.18 and fix golangci-linter warnings #146

Closed andig closed 2 years ago

andig commented 2 years ago

Go 1.12 is very old. This PR upgrades to Go 1.18 released in 2022. This PR fixes all linter warnings outside test and examples.

lorenzodonini commented 2 years ago

I completely agree that the minimum version set in go.mod is too generous and should be bumped at least to v1.15. But this one would break existing charge point implementations built with go <1.16 (os.ReadFile didn't exist back then). While this seems like a trivial change, it would possibly mean upgrading the go version on existing charging stations.

I personally don't mind upgrading on a regular basis, but hardware vendors typically don't like that 😕

andig commented 2 years ago

I'd argue that nobody has to upgrade... unless you want a newer version of this module ;) Go 1.16 would only be a year older, so not that much of a difference.

lorenzodonini commented 2 years ago

I'd argue that nobody has to upgrade... unless you want a newer version of this module ;)

Of course, but I also don't want to deny bugfixes to existing users or have to maintain different versions simultaneously tbh. Most libraries I know tend to keep backwards compatibility and upgrade the targeted version only if really needed.

How about we go for a middle ground and set the min version in go.mod to 1.16? The CI version could stay on v1.18 imho. I would feel much more at ease knowing that I'm enforcing a version from 2021 rather than forcing everyone to upgrade right now.

andig commented 2 years ago

I can change it to 1.16 but that would still deprecate ioutil. Main change reverted would be the go.mod format. Ok?

lorenzodonini commented 2 years ago

I can change it to 1.16 but that would still deprecate ioutil. Main change reverted would be the go.mod format. Ok?

Yes, that would be fine.

The deprecation itself doesn't worry me too much, since it came with 1.16 anyways. I'm more worried about changing the targeted version of the module, because that can break more than just a few CIs 😬

andig commented 2 years ago

Done. Please double-check the "ok" change in https://github.com/lorenzodonini/ocpp-go/pull/146/commits/5be6c76393bb0f576f6c54946a1d832746b78f8c.