trezor / trezord-go

:smiling_imp: Trezor Communication Daemon (written in Go)
GNU Lesser General Public License v3.0
245 stars 146 forks source link

gometalinter: CloseNotifier deprecated #138

Closed prusnak closed 5 years ago

prusnak commented 5 years ago
server/api/api.go:71:15:warning: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.  (SA1019) (staticcheck)
server/api/api.go:202:15:warning: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.  (SA1019) (staticcheck)
karelbilek commented 5 years ago

Fixing this will maybe break old go versions that we still support. (but I haven't looked that hard)

On Fri, Feb 15, 2019, 7:16 AM Pavol Rusnak <notifications@github.com wrote:

server/api/api.go:71:15:warning: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead. (SA1019) (staticcheck) server/api/api.go:202:15:warning: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead. (SA1019) (staticcheck)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/trezor/trezord-go/issues/138, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGZ8Ts_NL2LykUjgPVelUITF5IiUMPlks5vNfxDgaJpZM4a8sVq .

prusnak commented 5 years ago

I am fine with keeping this as is. Just created an issue, so we know why the gometalinter test is failing.

karelbilek commented 5 years ago

My own q/a on SO :)

https://stackoverflow.com/questions/54890809/how-to-use-request-context-instead-of-closenotifier/54890870#54890870

prusnak commented 5 years ago

So is this change backwards compatible or in case it is not, how old versions will this break?

karelbilek commented 5 years ago

Looking at travis results from #146 and the go release notes, it seems it was introduced in 1.7; so all our tests are still working and #146 can be merged

karelbilek commented 5 years ago

Fixed by #146