Conversation
go vet notes that using non-pointer methods copies the mutext lock that is a member of this class.
Otherwise go test -race may report that checkRedirect is read before it is written.
|
Looks good! Bit worried about the race condition fix, as users who had counted on SubscribeWith() following redirects will now have to configure their http.Client themselves: |
|
I moved the redirecting logic into a new |
|
If a user has created a |
|
Actually seems like copying headers on redirect was added in Go 1.8: https://golang.org/doc/go1.8#net_http Tricky decision is whether to remove the redirect functionality, and document Go 1.8 as a minimum requirement or to leave in backwards compatibility. |
Here are a few fixes to satisfy go vet and test. I'm running these on circleci. We use them regularly and it might be worth enabling them for this repo (maybe even adding a badge). There is also travis if you'd prefer that. You can see the failures this fixes at:
https://circleci.com/gh/launchdarkly/eventsource/19