Skip to content

Use AFMotion instead of Bubblewrap for HTTP#23

Open
killion wants to merge 9 commits intotkadauke:masterfrom
killion:master
Open

Use AFMotion instead of Bubblewrap for HTTP#23
killion wants to merge 9 commits intotkadauke:masterfrom
killion:master

Conversation

@killion
Copy link

@killion killion commented May 9, 2014

This request is because Bubblewrap is removing their HTTP module:
rubymotion-community/BubbleWrap#308

I tried to change as little as possible, but "success" is returned by AFMotion instead of "ok" like Bubblewrap. So this is a breaking change. I could create a pull request on AFMotion to add an alias but "success" is actually more correct.

The specs are broken because Webstub does not support AFMotion. It's noted here:
nathankot/webstub#20
I tested with my app and the example app and I haven't found any issues. Not sure what to do about it.

It would be better to move to AFMotion's JSON parser and remove Bubblewrap entirely but that is a bigger change that I didn't want to tackle.

@tkadauke
Copy link
Owner

tkadauke commented May 9, 2014

First of all, thanks for that pull request. I like the idea of moving towards AFMotion.

  • How about aliasing ok? to success? as a monkey patch (with a deprecation warning) in the AFMotion::HTTP::Response class provided by AFMotion?
  • I don't think that failing tests should ever be merged into master. Can you maybe rewrite them and stub them in a different way? I'm sure there must be a way to stub out AFMotion HTTP calls.
  • Please don't commit changes to the version number, that's the maintainer's task. If you need to use your particular version in your app, just reference the git commit SHA in your Gemfile.

Other than that, it looks great! :-)

@mnin
Copy link
Contributor

mnin commented Jun 12, 2014

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants