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
docs: Add documentation for outgoing webhooks. #9462
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be mostly from the other PR, but I made some comments on some minor points.
@@ -0,0 +1,114 @@ | |||
# Outgoing Webhooks | |||
|
|||
Outgoing webhooks are a type of integration where the Zulip server sends HTTP POST requests to a third party URL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this is currently POST requests, but need it be limited to this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outgoing webhooks are meant for POST requests only majorly (even Slack, Mattermost supports only POST requests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a tiny point, I just wondered if it need be limited to POST
, eg. if a different interface was written, and whether it could more broadly be "sends HTTP requests to a third party URL."
docs/outgoing-webhooks.md
Outdated
## Interfaces | ||
These are classes which customise the data to be posted to base URL of outgoing webhooks. These also provide | ||
methods to prepare data to be posted and help parse the response from the URL and generate the response | ||
to be sent to the user. For specialised purpose, user can create his own interface, else he can use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a long sentence - perhaps use more commas? ("These also provide..." sentence)
In the next sentence, you could use "they" rather than "he" to stay neutral, as per the rest of the document ("user", "you").
docs/outgoing-webhooks.md
Outdated
These are classes which customise the data to be posted to base URL of outgoing webhooks. These also provide | ||
methods to prepare data to be posted and help parse the response from the URL and generate the response | ||
to be sent to the user. For specialised purpose, user can create his own interface, else he can use | ||
the above interfaces supported by zulip. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "following", rather than "above"?
docs/outgoing-webhooks.md
Outdated
|
||
* **Generic** | ||
|
||
It is useful for general purpose webhooks. It can also be used for zulip bot server. It posts the the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeated "the" on this line :)
@neiljp Yes, this was majorly to resurrect the other PR as it was closed without the outgoing webhooks documentation being merged. I have updated the PR with the suggested changes! |
Not sure why the circleCI didn't run to completion - I had a look before and it doesn't seem related? |
With minor changes from @rheaparekh
These docs should move to Merged as cc0d0b5 after making those changes. I think there's more work to be done on this (e.g. just reading the request/reply section, it's pretty unclear how this works, and I'm not sure whether the API is even correct anyway). Thanks for all your work on this @rheaparekh! @eeshangarg FYI -- can you work on making the |
This is a follow-up to zulip#9462.
Resurrecting outgoing webhooks documentation from PR #5872, as the code has been merged but the documentation hasn't.