Skip to content
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

Added params to fql query method #47

Merged
merged 2 commits into from
Mar 20, 2014
Merged

Added params to fql query method #47

merged 2 commits into from
Mar 20, 2014

Conversation

rob0rt
Copy link
Contributor

@rob0rt rob0rt commented Mar 18, 2014

Hello!

First of all, thank you for this library, it's incredibly easy to use and very helpful.

I'm writing an application which requires an access token to be provided per-request for FQL queries, which I noticed your application does not support, since appending it to the query string makes the ampersand get eaten by the encodeURIComponent method. Since the FQL method simply calls the get method, I realized adding this support would not be complicated, so I went ahead and added it in my fork (so I myself could use it). This makes the FQL method signature closer to the get method signature, but I did add in a check to make sure the modifications are backwards compatible. In the event this is useful for you, I have opened this pull request.

Thanks!

@thinkloop
Copy link

I was just about to implement something similar before checking here. The code looks good, I hope it gets merged in. +1.

@BobertForever what's a good way to apply this change to my local node project before it's merged?

@rob0rt
Copy link
Contributor Author

rob0rt commented Mar 20, 2014

@thinkloop If you are using a package.json file, simply doing something like the following will work:

{
    "name": "your-app",
    "version": "0.0.1",
    "main": "app.js",
    "dependencies": {
        "fbgraph": "BobertForever/fbgraph"
    }
}

If you are installing dependencies manually using npm, running

npm install BobertForever/fbgraph

should do the trick

@thinkloop
Copy link

Works like a charm! Thank you sir.

@criso
Copy link
Owner

criso commented Mar 20, 2014

Awesome! Please add to the fql section on the docs and I'll merge it after that

@rob0rt
Copy link
Contributor Author

rob0rt commented Mar 20, 2014

@criso I've added a new segment to the docs, my pull request has updated with the changes

criso added a commit that referenced this pull request Mar 20, 2014
Added params to fql query method
@criso criso merged commit 7fc899f into criso:master Mar 20, 2014
@criso
Copy link
Owner

criso commented Mar 20, 2014

Nice!

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