EmbarkStudios / octobors

Rust program for automerging PRs based on a few rules
Apache License 2.0
40 stars 2 forks source link

Try to maintain `Fixes #...` messages from squashed commits? #20

Open bnjbvr opened 3 years ago

bnjbvr commented 3 years ago

When working a pull request, developers may embed Fixes #... annotations within commit messages. Right now it seems that when octobors squashes commits, it uses the pull request's description as the commit message. As a result this will lose the annotations from the commit messages by default. It would be nice if it had the ability to parse the commit messages from the pull request, and keep these annotations in the final squashed commit's message.

repi commented 3 years ago

We prioritised getting the PR description for the squash commits as that is the main thing one has when the branch has multiple commits.

Could be possible for single-commit branches/merges to somehow combine the PR description with the commit description, or just use the commit description - but we often had more detailed PR descriptions than the commit descriptions so does sound a bit tricky and that one may have to just pick either or.

So the PR should have the "Fixes XX" strings also right, as otherwise the PR won't be linked and close the issues it is fixing, are you saying here that Octobers removes those lines from the PR description?

bnjbvr commented 3 years ago

So the PR should have the "Fixes XX" strings also right, as otherwise the PR won't be linked and close the issues it is fixing, are you saying here that Octobers removes those lines from the PR description?

Not quite. In my case, the PR didn't have the Fixes XX string, but one commit in the PR contained it in its commit message. The commit messages are dropped when squashing, and Github doesn't propagate the Fixes XX annotations from the commit messages to the pull request's description automatically, so the Fixes XX was lost when squashing and merging.

repi commented 3 years ago

Ah but don't you have to have the "Fixes XX" in the PR description though for GitHub to link the PR with the issue, and close the issue when the PR is merged? Or does it find that a commit in the PR branch referenced it and close the referred issue when it is merged?

Jake-Shadle commented 3 years ago

AFAIR github will only auto close if it's in the PR description, it will only show that an issue was referred from via a commit.

bnjbvr commented 3 years ago

It also works when the Fixes XXX annotation is contained in commit messages of a PR, and that is the way I've used it for years. See example repro, where this pull request contained two commits, one of which had a commit message saying it'll fixes this issue, which caused the issue to be closed when merging the PR.

repi commented 3 years ago

Seems to partially work, those commits in the PR didn't make the PR list the issues that it will resolve in the linked issues list:

image

So do think it would be safer to rely on mentioning issues in the PR description directly still.