search

pion / dtls

DTLS 1.2 Server/Client implementation for Go
https://pion.ly/
MIT License
602 stars 160 forks source link

Use atomic.Value to maintain Go 1.13 compatibility #582

Closed hasheddan closed 1 year ago

hasheddan commented 1 year ago

Description

Updates to use atomic.Value in place of atomic.Bool such that older Go versions can continue to be supported.

Signed-off-by: Daniel Mangum georgedanielmangum@gmail.com

Similar fix to https://github.com/pion/transport/commit/701ff64e8ae945808b3e82c783e37d514a731c44

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage is 66.66% of modified lines.

Files Changed Coverage
internal/net/udp/packet_conn.go 66.66%

:loudspeaker: Thoughts on this report? Let us know!.

daenney commented 1 year ago

Honestly, I'd rather we don't and update the minimum version in go.mod instead.

Go 1.13 is from September 2019. It's now 4 years later. Go is very forwards compatible so the burden of upgrading a version once a while, especially once every 4 years, is basically 0. Go 1.19, which is what we'd need for atomic.Bool, is already a year old and with the release of 1.21 already no longer supported by the Go team. I think it's fine to move our baseline to it. That would also enable the use of generics if we want it at some point.

daenney commented 1 year ago

Go 1.13 is from September 2019. It's now 4 years later. Go is very forwards compatible so the burden of upgrading a version once a while, especially once every 4 years, is basically 0. Go 1.19, which is what we'd need for atomic.Bool, is already a year old and with the release of 1.21 already no longer supported by the Go team. I think it's fine to move our baseline to it. That would also enable the use of generics if we want it at some point.

@daenney I tend to agree, but I believe there is a desire for other pion libraries to continue to support older Go versions, and other consumers are having to workaround this to maintain their older version support as well (e.g. plgd-dev/go-coap#481). Perhaps it makes more sense to maintain the compatibility for now, then start a discussion about what our minimum Go version requirement policy will be going forward?

Yeah. I think that's a problem though since our CI also doesn't test those things anymore. But yup, lets have that discussion after this.

at-wat commented 1 year ago