search

facebookarchive / fbconsole

A micro api client for writing scripts against the Facebook Graph API.
Other
325 stars 74 forks source link

Platform API version now explicit and defaults to v2.0, as mentioned in #33 #36

Open kchr opened 9 years ago

kchr commented 9 years ago

Added explicit reference to platform API version, as introduced by Facebook in version 2.0 (Issue #33)

facebook-github-bot commented 9 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

facebook-github-bot commented 9 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

psineur commented 9 years ago

Ok, so as I commented on #33 - v1.0 being deprecated doesn't break the fbconsole. Unversioned requests are just being defaulted to v2.0

I think top priority right now will be #37 or related Python 2/3 Unix/Windows bugs that we have that might be fixed with this.

I definitely want to bring better support of versioning to fbconsole, so here's the idea:

  1. Default to last available version 2.3
  2. add version property that will support something like this:
fbconsole.setVersion(2.3) - I like this one cause it's easier to type.
fbconsole.setVersion(2) - shortcut for 2.0
fbconsole.setVersion('2.3') - usually we set versions as strings in other SDKs
fbconsole.setVersion('v2.3')

  1. Allow overwriting version in request path:

    fbconsole.get('/me') should use the set version, but fbconsole.get('/v2.1/me') overrides set version to 2.1 for this request

3.1 Fix get('me') failing - just ensure that path starts with slash or use URI lib / better logic there.

What do you think? If this is fine with you and you want to update your pull request - I'm happy to merge it as soon as possible. If you don't have time for this - it's fine, I guess I will start looking into versioning right after #37, so I guess I will close your issue as needs-no-action now.

kchr commented 9 years ago

Hi Stepan!

Thanks for the feedback, that sounds like a better long-term solution, with less impact on the end users who already did the switch in their request URIs (specifying version on their own). I'd be glad to adapt the solution this way; just need a few more days to sort out some other projects.

Expect an update during the upcoming week or so!

And thanks for the API work, keep it up. :+1:

-- kchr

psineur commented 9 years ago

We're going to use Phabricator-style tags for Pull Request status:

Please change GH Review: needs revision to GH Review: needs review after updating the Pull Requests.

Hope this will not bother people too much. Cheers!

ghost commented 9 years ago

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.