Skip to content

Conversation

Britaliope
Copy link

@Britaliope Britaliope commented Mar 15, 2025

Summary

Documentation MR for mattermost/mattermost#30495

Question: Does it makes sense to have the webhook documentation on https://developers.mattermost.com ? My first instinct when looking for this documentation was to look on https://api.mattermost.com

- Webhook can now post in threads
- Webhook now returns a JSON representation of the message instead of "OK"
@mattermost-build
Copy link
Contributor

Hello @Britaliope,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@cwarnermm cwarnermm requested review from a team and davidkrauser and removed request for a team March 17, 2025 12:48
@cwarnermm
Copy link
Member

@davidkrauser - Assigned you as a member of core reviewers. Feel free to reassign as needed.

Copy link
Contributor

@davidkrauser davidkrauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to update the documentation along with your change 🙂

@Britaliope wrote:

Question: Does it makes sense to have the webhook documentation on https://developers.mattermost.com/ ? My first instinct when looking for this documentation was to look on https://api.mattermost.com/

I think what you've done is right - the documentation at api.mattermost.com specifies the endpoints to create and manage webhooks, while the webhook documentation on developers.mattermost.com specifies how to use them once created.

@cwarnermm cwarnermm self-requested a review March 19, 2025 16:39
@cwarnermm cwarnermm added 1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor labels Mar 19, 2025
Copy link
Contributor

@davidkrauser davidkrauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, pending the related PR merges: mattermost/mattermost#30495

@cwarnermm this introduces a minor breaking change to the API. Instead of returning the plain text ok, we now return the contents of the post that was created. I know you're working on a new process around breaking changes, so wanted to point it out 🙂

@davidkrauser davidkrauser removed the 1: Dev Review Requires review by a core commiter label Mar 19, 2025
@cwarnermm
Copy link
Member

Really appreciate the visibility on this, @davidkrauser! Thank you!

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting your changes!

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@cwarnermm cwarnermm requested a review from hanzei April 14, 2025 13:10
@hanzei hanzei removed their request for review April 14, 2025 14:00
@hanzei hanzei added Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) and removed Lifecycle/1:stale labels Apr 17, 2025
@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Editor Review Requires review by an editor Contributor Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) Lifecycle/1:stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants