Faraday 0.8 is compatible with current usage, so why not allowing to use it? #34

Merged
merged 4 commits into from Jul 13, 2012

5 participants

@rewritten

The only relevant change is in the signature of Connection#get et al. when headers are in the arguments (second arg in 0.7, third arg in 0.8). The block form Connection.get { |req| .... } has the same usage in both versions, and this is the form used in the code.

@trafnar

You don't get undefined method `query_values' for #<URI::HTTPS:0x007ff5f24b1618> errors when using Faraday 0.8?

@rewritten

Whoa, that was a huge mistest, I didn't enforce 0.8 version when running the tests. Now the source of the missing method (dropping automatic usage of Addressable::URI) is solved by converting the uri into an addressable one.

I have a lot of missing stubs now. As soon as they are fixed (so both with 0.7.6 and 0.8.0), i'll push again and ping you...

@rewritten

ping

@trafnar

Works for me! +1

@rewritten

Hey, ma2gedev's commit is much better in my opinion (for what concerns the oauth middleware, as it is not forcing the use of Addressable, using only Faraday helpers. I'll add (sort of) his implementation over here, then you choose what to integrate.

@rewritten

I removed the need of Addressable::URI, using the Faraday::Utils methods, but unlike in ma2gedev's commit, the query is correctly reset to nil when empty (so tests pass) and there is less changed code.

@rewritten

By the way, all this would be totally useless if Instagram supported having OAuth2 data in headers only (I'm sure the specification don't require both!) so we could use a much simpler auth middleware like the one provided in next faraday_middleware release (builder.use :auth, "Token", :token => access_token)

@yaauie yaauie added a commit to simplymeasured/instagram-ruby-gem that referenced this pull request Jul 5, 2012
@yaauie yaauie merge pull #34 from @rewritten to handle Faraday >= 0.8 d962295
@yaauie

+1 on getting this merged into master. I just merged it into our shallow-fork.

@luke-gru

+1 to merge into master please.

@shayne shayne merged commit 3bab309 into facebookarchive:master Jul 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment