Multiple improvements to GitLab push event formatting

[HarHarLinks]

This PR accomplishes 2 these changes to the rendering of GitLab events:

• commit hashes formatted as inline code
• "N commits" links to the list of commits on that branch instead of just the last commit individually
• only show list of commits if it is complete
• list commiter names unless all commits were authored and pushed by the same person (identified by email)
• open issue about configuring max length of commits list
• open issue for configuring <details> commit lists

I also want to discuss:

• Why format the branch name as inline code?
• You limit the amount of commits to the last 5 in case of pushing more that that at once. I can see the reasoning behind that, you might not want to spam the room with loads of commits when you subscribe to pushes in order to be notified that there was a push. As reader you can infer that some are hidden by the total number of commits. Alternatively you could only include the list if it is "complete" and not include a list but only the link to "N commits" when it wouldn't be.
• On top of this, GitLab limits itself to including at most 20 commits in a webhook anyway. I would like the bridge to remind me in these cases that while it says "pushed 20 commits" there may actually be more, i.e. say "pushed 20+ commits" or similar.
• Personally my workflow using GitLab webhooks (with the old bridge and some jinja) has been to review all of the commit messages on matrix, because after all I want to be lazy and not have to click these links. 😄 How hard would it be to make the limit of 5 commits configurable?
• Can't you have multiple gitlab hooks per room? If you can, then perhaps the repo should be more prominent, e.g. **strong**, hyperlinked and in the first line before everything else, like a heading (but an actual <h1> would be too much)

Signed-off-by: Kim Brose kim.brose@rwth-aachen.de

[Half-Shot]

Sorry, pressed the button without thinking

[HarHarLinks]

Sorry, pressed the button without thinking

Yeah, it is "Ready for review" in the sense that I want you to look at it, I drafted because I wanted to discuss before merging, which we don't have a button for.

[Half-Shot]

Why format the branch name as inline code?

I copied it from the tag push code I suspect. I don't really have an opinion on keeping it that way.

You limit the amount of commits to the last 5 in case of pushing more that that at once. I can see the reasoning behind that, you might not want to spam the room with loads of commits when you subscribe to pushes in order to be notified that there was a push. As reader you can infer that some are hidden by the total number of commits. Alternatively you could only include the list if it is "complete" and not include a list but only the link to "N commits" when it wouldn't be.

I would be okay with doing that, but maybe include the last commit so there is "some" context about what happened (e.g. a merge, fix, whatever). Possibly the same behaviour as with 1 commit, but with a link to the rest.

On top of this, GitLab limits itself to including at most 20 commits in a webhook anyway. I would like the bridge to remind me in these cases that while it says "pushed 20 commits" there may actually be more, i.e. say "pushed 20+ commits" or similar.

GitLab should still give you the total commit count (in total_commits_count), so we should probably use that when doing the above behavior.

Personally my workflow using GitLab webhooks (with the old bridge and some jinja) has been to review all of the commit messages on matrix, because after all I want to be lazy and not have to click these links. smile How hard would it be to make the limit of 5 commits configurable?

Happy for you to make that configurable via room state :)

Can't you have multiple gitlab hooks per room? If you can, then perhaps the repo should be more prominent, e.g. strong, hyperlinked and in the first line before everything else, like a heading (but an actual

would be too much)

I think there might be some prior art in the GitHubRepo code you could use as inspiration?

Having it linked felt prominent enough to me.

[HarHarLinks]

You limit the amount of commits to the last 5 in case of pushing more that that at once. I can see the reasoning behind that, you might not want to spam the room with loads of commits when you subscribe to pushes in order to be notified that there was a push. As reader you can infer that some are hidden by the total number of commits. Alternatively you could only include the list if it is "complete" and not include a list but only the link to "N commits" when it wouldn't be.

I would be okay with doing that, but maybe include the last commit so there is "some" context about what happened (e.g. a merge, fix, whatever). Possibly the same behaviour as with 1 commit, but with a link to the rest.

This sounds good

On top of this, GitLab limits itself to including at most 20 commits in a webhook anyway. I would like the bridge to remind me in these cases that while it says "pushed 20 commits" there may actually be more, i.e. say "pushed 20+ commits" or similar.

GitLab should still give you the total commit count (in total_commits_count), so we should probably use that when doing the above behavior.

I somehow missed this field existed, excellent.

Personally my workflow using GitLab webhooks (with the old bridge and some jinja) has been to review all of the commit messages on matrix, because after all I want to be lazy and not have to click these links. smile How hard would it be to make the limit of 5 commits configurable?

Happy for you to make that configurable via room state :)

Oof ok I declare that beyond the scope of this quick PR but maybe eventually when it's rusted so I can tackle that and learn rust instead of js 😜

Can't you have multiple gitlab hooks per room? If you can, then perhaps the repo should be more prominent, e.g. strong, hyperlinked and in the first line before everything else, like a heading (but an actual # would be too much)

I think there might be some prior art in the GitHubRepo code you could use as inspiration?

Having it linked felt prominent enough to me.

I don't think I understand what you're saying. Anyway here is a comparison of a GitLab push using

1. my jinja. Somehow this order makes more sense to me, but it may or not have been based on 3.
   

2. current hookshot i.e. before this PR
   

3. Whatever nheko uses (maubot-gitlab thx @deepbluev7)
   

Anyway, doing this here would mean all the other events should follow this scheme as well, but for this consistency alone it might be worth it.

[HarHarLinks]

Re: lists of commits, I've seen https://matrix.to/#/#android-announcements:schildi.chat use <details> (which is supported by matrix spec, but far from implemented in all clients)

[Half-Shot]

<details> should be fine, happy for you to use that. I suppose it depends what the fallback is for clients that don't support it (would it omit the body, might not be the end of the world if we link to the full changeset)

I'm wondering if the repo prominence thing could be fixed in a separate PR?

[deepbluev7]

(nheko uses maubot-gitlab)

[Half-Shot]

Seems reasonable otherwise.

[Half-Shot]

@HarHarLinks can you make the title more helpful too :)

[HarHarLinks]

(CI is broken because current main is broken which I merged due to merge conflict. I can merge main again once it's fixed if you like)