-
Notifications
You must be signed in to change notification settings - Fork 324
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
Webmock broken by new HTTP::Response#initialize API #277
Comments
Sounds good. I'd like to see us do this in a way that's more backwards compatible but I'm not exactly sure if that's feasible. |
Oh. I merged that and forgot to provide webmock adapter update. Will do that this weekend. |
I don't see a way to make it backward compatible either. But I don't see worth in keeping API backward compatible - we just removed bunch of methods ;D we're braking bad already :D |
Sidebar: if we do this for https://github.com/httprb/http/blob/master/lib/http/request.rb#L67 |
Agree! |
I released 1.0.0.pre4 with the updated @ixti want to take a crack at fixing Webmock? |
Yeah. Sure. Will prepare a PR for webmock today. Sure. |
PR for webmock is: bblimke/webmock#554 That will make webmock adapter always up to date with http API. |
In theory this should be the last breaking change because the 1.0 API should be stable, so I'm not sure that matters? Perhaps we should move it the next time we feel the need to change it. |
Agree! |
See f40fbda: Use opts hash for HTTP::Response.new
This seems like a good change. I think we should release with this new API and update Webmock accordingly.
cc @Connorhd @nerdrew @zanker
The text was updated successfully, but these errors were encountered: