Donovan Hide

Results 28 comments of Donovan Hide

This is too big a change to accept without added unit tests that prove current behaviour is not subtly altered.

Thanks! I guess to be thorough you'd need to protect this send to a channel as well: https://github.com/aphistic/eventsource/blob/06183ec6ef439852031159a0ee9ba8f956138dfc/stream.go#L209 It does seem that mutex is being referred to a lot :-)...

Two PRS for the price of one :-) https://github.com/donovanhide/eventsource/pull/36 To be honest, I don't actually use this package at all anymore. My only concern is that changes affect users in...

Yep, this library was written well before context.Context had even been imagined :-) You have my blessing to rip out any parts of this library that are useful and I'll...

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: https://github.com/donovanhide/eventsource/pull/35/commits/6fc0f23cc4a72344ced8336883bdaf4439f2f9bd#diff-e2d91404d5609adc9273e4603f6ce67dL73

If a user has created a `Stream` with `SubscribeWith` and provided their own `http.Client`, then with this patch, the `Client` will not have its `CheckRedirect` func set to our `checkRedirect`...

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...

@nbougalis Another one for you?

Looks like this might have been the copy paste source :-) https://stackoverflow.com/questions/2883164/openssl-certificate-lacks-key-identifiers

Simplest fix for this is to simply delete the erroneous line above and accept the default version (`X509_VERSION_1`): https://www.openssl.org/docs/man3.0/man3/X509_set_version.html Perhaps this is a good lesson in checking return codes from...