mattermost / mattermost-plugin-gitlab

GitLab plugin for Mattermost
Apache License 2.0
137 stars 83 forks source link

Unitfy webhook event structure #83

Closed hanzei closed 1 year ago

hanzei commented 5 years ago

The structure of the webhook events messages isn't very consistent. E.g.:

Comment on an issue:

$REPO New comment by $AUTHOR on $ISSUE_NUMBER $ISSUE_NAME
$CONTENT

Close issue:

[$REPO] Issue $ISSUE_NUMBER closed by $AUTHOR`

Opening an issue:

#### $ISSUE_NAME

##### $REPO/$ISSUE_NUMBER

# new issue by $AUTHOR on $TIMESTAP

MR events are structure similarly.

My proposal would be:

  1. Start comment and close event with [$REPO] and link to the repo
  2. Don't show issues numbers at all
  3. Don't show timestamps at all
  4. Always use the username
  5. Add @ before username
  6. Use #### as the maximum heading
hanzei commented 5 years ago

@manland Thoughts on this?

manland commented 5 years ago

@hanzei I agry with you all events are not consistent!

But I don't know if users prefer short version (1 line) or long version (with details)!

I had read somewhere a work in progress about adding more details in a message. With a little info icon at the right of the message and when user chick on it more details are displayed.

With this feature we could make all messages 1 line and hide details in this new view.

But your right we can start a refactor step before adding this kind of feature. Do we need to synchronise with GitHub plugin?

hanzei commented 5 years ago

But I don't know if users prefer short version (1 line) or long version (with details)!

I'm fine with showing the content for comments or for new issues.

I had read somewhere a work in progress about adding more details in a message. With a little info icon at the right of the message and when user chick on it more details are displayed.

Can you link something about this topic? I haven't heared about this. Still, this might be handy in the future, but not now.

Do we need to synchronise with GitHub plugin?

Ideas 1, 4, 5 and 6 are taken from the GitHub plugin. 2 3 are not, but people talked about applying these changes to the GitHub plugin.

manland commented 5 years ago

Hi @hanzei sorry for delay I was in holidays ;)

I had read somewhere a work in progress about adding more details in a message. With a little info icon at the right of the message and when user chick on it more details are displayed.

Can you link something about this topic? I haven't heared about this. Still, this might be handy in the future, but not now.

Sure https://community.mattermost.com/core/pl/rp7aephtqbr5p8rsmpoq86g49c

@jasonblais have you created issue related to this idea? Do you know if something as started about that?

jasonblais commented 5 years ago

@manland Are you referring to work like this: https://github.com/mattermost/mattermost-plugin-github/pull/103

Hope you had a nice holiday! :)

manland commented 5 years ago

@jasonblais not exactly this, but I wait to see this on community! I love it!

I spoke about that https://community.mattermost.com/core/pl/rp7aephtqbr5p8rsmpoq86g49c but maybe it hasn't much more attraction for other plugins?

Thanks holidays was really good 😎🎉😀👌

jasonblais commented 5 years ago

Got it! Support for that feature was added and is available on 5.14. However, the mobile aspect is still a work in progress.

Here's the PR that added support for it (https://github.com/mattermost/mattermost-webapp/pull/2305) and https://github.com/mattermost/mattermost-plugin-github/pull/103 is indeed the first plugin feature that's using it, soon to be on community, and hopefully soon for the GitLab plugin as well! :)

manland commented 5 years ago

Ok yes, got it too! The implementation in the github-plugin is not exactly what I was thinking, but ok I think I have an idea to make it work. When I have finished #53 i'll give it a try ;)

ohporter commented 5 years ago

@hanzei I agry with you all events are not consistent!

Agreed, and this is actually the reason why my company doesn't yet use the subscribe feature in this plugin. The gitlab.com Mattermost integrations generate a very consistent (and arguably visually pleasing) event post...although at the expense of flexibility of the plugin's subscribe command.

But I don't know if users prefer short version (1 line) or long version (with details)!

If there's ever a policy decision on what users prefer, I would recommend that it end up as a configurable option. There's a lot of places in MM where policy is still hardcoded (e.g. MAX_POST_HEIGHT) and ideally it would be good not to add to it.

I had read somewhere a work in progress about adding more details in a message. With a little info icon at the right of the message and when user chick on it more details are displayed.

With this feature we could make all messages 1 line and hide details in this new view.

This would be ideal from my POV, given then RHS feature I see discussed later in this thread. If you do end up with an implementation that is deciding between 1-2 lines compact post and many lines of info in one post, it really would help to make that default a configurable option (and ideally a per-user display option). May be a moot point with an implementation that expands the details into RHS.

wiersgallak commented 1 year ago

Closing due to inactivity. Issue can be reopened with more interest from our community.