futurelearn / squiddy

Github Actions bot
1 stars 1 forks source link

Improve Squiddy's merge bubble commit message #7

Closed marcotranchino closed 4 years ago

marcotranchino commented 4 years ago

Squiddy's Bubble Merge action is almost ready to be used in our main repo.

While we continue testing it thoroughly, we would also like to improve the automatic message associated with the merge commit by Squiddy.

The top part of the commit message is automatically generated by Squiddy and contains:

The bottom part of the commit message contains the entire text of the PR comment containing the keywords triggering the merge action. See images below.

PR Comment triggering the action

PR comment

Commit message

Merge commit message

Feedback

Any suggestions for changes?

jamiecobbett commented 4 years ago

I can't see anything else, and I think this is a definite improvement, thanks so much for doing this ❤️ 🦑

marcotranchino commented 4 years ago

New commit message format with linked PR

latest version with PR link

surminus commented 4 years ago

If this is ready to go then we should merge it in.

I'm still concerned by the amount of time it takes to run, which is roughly 1 minute 15 seconds, and the majority of that time is building our action, which takes ~30 seconds. Using the pre-built container (see this example).

I said before I think it's fine to iterate and improve later on, but I just wanted to officially mark it down as something we should look at.

marcotranchino commented 4 years ago

Agree. We could merge this in. I am just wondering if we should link the PR also in the title of the commit or just within the commit message. In a list of commits, the titles are usually linked to the commit themselves, but if we link the PR, then parts of the title link to the commit, and the PR number links to the PR. Is that confusing? Or OK?