socketry / protocol-http2

MIT License
9 stars 10 forks source link

support 2.3 on CI #2

Closed ganmacs closed 5 years ago

ganmacs commented 5 years ago

ref https://github.com/socketry/protocol-http/pull/5#issuecomment-503583836

ioquatix commented 5 years ago

Hmm, looks like unpack1 is undefined on 2.3...

The previous way is unpack(...).first.

ioquatix commented 5 years ago

I am happy to go back to unpack(...).first but I start asking myself is it really good idea? It's slower, Ruby 2.3 is EOL, etc. I know for some business it's important.

Maybe simpler option is to have monkey patch for backwards compatibility, e.g. if ruby is 2.3, define unpack1, etc. What do you think?

ganmacs commented 5 years ago

Maybe simpler option is to have monkey patch for backwards compatibility, e.g. if ruby is 2.3, define unpack1, etc. What do you think?

looks good. I'll do that.

ganmacs commented 5 years ago

@ioquatix friendly ping

ioquatix commented 5 years ago

Thank you for friendly ping. I will review this weekend.

ioquatix commented 5 years ago

Thanks for your effort here.

ioquatix commented 5 years ago

@ganmacs there are more issues to sort out: https://github.com/socketry/async-http/issues/24