-
Notifications
You must be signed in to change notification settings - Fork 63
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 functionality for FB messenger API #48
base: master
Are you sure you want to change the base?
Conversation
Why did you chose to put |
You need a specialised If you're okay with changing the existing |
I see. I wonder if we couldn't just set that Content Type by default on the |
We can do it if you're ok with changing all calls to If we add a parameter with a default value It wants:
But the problem with that is you get the default for headers but the top bodyless clause arguments are out of sync with the arguments of And if we set the headers inside the function body then we're locked in to always using JSON. The messenger API doesn't take anything else, but other endpoints might. |
Sorry for replying late.
Let's do that. 👍 |
In #44 I brought up the idea of replacing the I was procrastinating on making these changes until the open PR's were merged, but given this could potentially save some effort here, I will make something public sooner. |
@Potrimpo could you rebase your changes on master please? |
ping? |
@Potrimpo could you please update your PR to the latest changes? Otherwise I'll have to close this PR. |
addition to public API for talking to Messenger.
I can rename to
post(:msg, ...)
to be the same as the existingpost(:video, ...)
if that's hugely preferable, but personally I think separating the namespace is cleaner