-
Notifications
You must be signed in to change notification settings - Fork 10
Remove Async::IO dependency.
#32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Okay this is now working for me locally. |
bryanp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@ioquatix do you mind addressing the linter failure? https://github.com/bryanp/llhttp/actions/runs/13664554845/job/38204178670?pr=32 |
|
Yes sure, those lines were me debugging the server tests. I've removed them. |
|
This PR also changes the minimum Ruby version. Hopefully that is okay? |
|
Can it be dropped down to 3.0? This gem is used by http.rb which still depends on 3.0: https://github.com/httprb/http/blob/1af3c8e2dfb84d73e7febaaffce7bd7f4829cced/http.gemspec#L28 If that's an issue I can open a PR over there to try and bump it next release. |
|
My suggestion would be to skip those tests on older Ruby versions, as there is nothing intrinsically problematic about the code in |
|
The issue is that you bumped the minimum ruby version in the gemspec to 3.1—doesn't that make new releases of this gem incompatible with http.rb? |
|
Yes, you'd need to revert the changes I made to the gemspec, and then only pull in async for the tests on Ruby 3.1+. Are you able to get this PR across the finish line? |
|
Yeah will do, thanks again for your help. |
864967f to
4992e19
Compare
|
Great! Well done. |
No description provided.