Skip to content

Commit

Permalink
Hint at how to properly close issues with pull requests (opencast#5257)
Browse files Browse the repository at this point in the history
Mentioning issues in your PR with some magic keywords can automatically
close these issues when the PR is merged. We even hint at this in our
PR-template already.

However, GitHubs implementation of this does not work super well with
our release/branching model. Specifically, it only works if the PR is
merged into the default branch, which is `develop` in our case.

It **can** be made to work in our situation if the magic phrases are
mentioned in commit messages alternatively or in addition to the PR
description, because we regularly forward merge the release branches.

This patch adds a corresponding note about this to the documentation,
and links it in the PR template.
  • Loading branch information
JulianKniephoff authored Jan 31, 2024
2 parents 9a090d6 + be34b73 commit d9bb7c0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
### Your pull request should…

* [ ] have a concise title
* [ ] [close an accompanying issue](https://help.github.com/en/articles/closing-issues-using-keywords) if one exists
* [ ] [close an accompanying issue](https://docs.opencast.org/develop/developer/#participate/development-process/#automatically-closing-issues-when-a-pr-is-merged) if one exists
* [ ] [be against the correct branch](https://docs.opencast.org/develop/developer/development-process#acceptance-criteria-for-patches-in-different-versions)
* [ ] include migration scripts and documentation, if appropriate
* [ ] pass automated tests
Expand Down
27 changes: 27 additions & 0 deletions docs/guides/developer/docs/participate/development-process.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,33 @@ There are a couple of rules that committers must follow when merging pull reques
* It is advised to be pragmatic and only do so if necessary.


#### Automatically closing issues when a PR is merged

Our pull request template wants you to "close an accompanying issue."
This can be done as per the [GitHub documentation](https://help.github.com/en/articles/closing-issues-using-keywords)
by using one of several magic keywords in front of a valid issue number,
either in the pull request description, or in any of the commit messages
of the commits you want to merge. For example:

> This PR **fixes #1234**.
A word of caution: due to our [branching model](#git-repository-branching-model)
this might not always work as expected. GitHub only recognizes
the magic words when acting on the default branch of the repository,
which in our case is `develop`. Issues mentioned in descriptions
of PRs targeting `develop` or in any message of a commit that lands
in `develop` will be automatically closed.

Thus, if you are submitting a PR **not** targeting `develop`, and you
want to use this feature, you **have to** mention the magic words
in a commit message. Tne PR description does not work in this case.
And even then, the issue will only be closed, once your merged commits
reach `develop` by our forward merging process.

Mentioning related issues in the PR description **in addition** to
the commit message(-s) might of course still be useful for reviewers!


Git Repository Branching Model
------------------------------

Expand Down

0 comments on commit d9bb7c0

Please sign in to comment.