diff options
| -rw-r--r-- | .github/CONTRIBUTING.md | 67 | ||||
| -rw-r--r-- | .github/PULL_REQUEST_TEMPLATE.md | 1 | ||||
| -rw-r--r-- | Makefile.am | 8 | ||||
| -rwxr-xr-x | scripts/commitcheck.sh | 144 |
4 files changed, 219 insertions, 1 deletions
diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index a7d2bd4d7..3f0b4eadd 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -24,6 +24,9 @@ started?](#what-should-i-know-before-i-get-started) [Style Guides](#style-guides) * [Coding Conventions](#coding-conventions) + * [Commit Message Formats](#commit-message-formats) + * [New Changes](#new-changes) + * [OpenZFS Patch Ports](#openzfs-patch-ports) Helpful resources @@ -117,6 +120,8 @@ feature needed? What problem does it solve? without conflicts. * Please attempt to limit pull requests to a single commit which resolves one specific issue. +* Make sure your commit messages are in the correct format. See the +[Commit Message Formats](#commit-message-formats) section for more information. * When updating a pull request squash multiple commits by performing a [rebase](https://git-scm.com/docs/git-rebase) (squash). * For large pull requests consider structuring your changes as a stack of @@ -150,3 +155,65 @@ to verify ZFS is behaving as intended. We currently use [C Style and Coding Standards for SunOS](http://www.cis.upenn.edu/%7Elee/06cse480/data/cstyle.ms.pdf) as our coding convention. + +### Commit Message Formats +#### New Changes +Commit messages for new changes must meet the following guidelines: +* In 50 characters or less, provide a summary of the change as the +first line in the commit message. +* A body which provides a description of the change. If necessary, +please summarize important information such as why the proposed +approach was chosen or a brief description of the bug you are resolving. +Each line of the body must be 72 characters or less. +* The last line must be a `Signed-off-by:` line with the developer's +name followed by their email. + +Git can append the `Signed-off-by` line to your commit messages. Simply +provide the `-s` or `--signoff` option when performing a `git commit`. +For more information about writing commit messages, visit [How to Write +a Git Commit Message](https://chris.beams.io/posts/git-commit/). +An example commit message is provided below. + +``` +This line is a brief summary of your change + +Please provide at least a couple sentences describing the +change. If necessary, please summarize decisions such as +why the proposed approach was chosen or what bug you are +attempting to solve. + +Signed-off-by: Contributor <[email protected]> +``` + +#### OpenZFS Patch Ports +If you are porting an OpenZFS patch, the commit message must meet +the following guidelines: +* The first line must be the summary line from the OpenZFS commit. +It must begin with `OpenZFS dddd - ` where `dddd` is the OpenZFS issue number. +* Provides a `Authored by:` line to attribute the patch to the original author. +* Provides the `Reviewed by:` and `Approved by:` lines from the original +OpenZFS commit. +* Provides a `Ported-by:` line with the developer's name followed by +their email. +* Provides a `OpenZFS-issue:` line which is a link to the original illumos +issue. +* Provides a `OpenZFS-commit:` line which links back to the original OpenZFS +commit. +* If necessary, provide some porting notes to describe any deviations from +the original OpenZFS commit. + +An example OpenZFS patch port commit message is provided below. +``` +OpenZFS 1234 - Summary from the original OpenZFS commit + +Authored by: Original Author <[email protected]> +Reviewed by: Reviewer One <[email protected]> +Reviewed by: Reviewer Two <[email protected]> +Approved by: Approver One <[email protected]> +Ported-by: ZFS Contributor <[email protected]> + +Provide some porting notes here if necessary. + +OpenZFS-issue: https://www.illumos.org/issues/1234 +OpenZFS-commit: https://github.com/openzfs/openzfs/commit/abcd1234 +``` diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 7c11a46da..d577aaabe 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -29,4 +29,5 @@ - [ ] I have read the **CONTRIBUTING** document. - [ ] I have added tests to cover my changes. - [ ] All new and existing tests passed. +- [ ] All commit messages are properly formatted and contain `Signed-off-by`. - [ ] Change has been approved by a ZFS on Linux member. diff --git a/Makefile.am b/Makefile.am index 3adc161ad..cea264acf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,7 +39,12 @@ dist-hook: sed -i 's/Release:[[:print:]]*/Release: $(RELEASE)/' \ $(distdir)/META -checkstyle: cstyle shellcheck flake8 +checkstyle: cstyle shellcheck flake8 commitcheck + +commitcheck: + @if git rev-parse --git-dir > /dev/null 2>&1; then \ + scripts/commitcheck.sh; \ + fi cstyle: @find ${top_srcdir} -name '*.[hc]' ! -name 'zfs_config.*' \ @@ -51,6 +56,7 @@ shellcheck: scripts/zloop.sh \ scripts/zfs-tests.sh \ scripts/zfs.sh; \ + scripts/commitcheck.sh; \ (find cmd/zed/zed.d/*.sh -type f) | \ grep -v 'zfs-script-config' | \ while read file; do \ diff --git a/scripts/commitcheck.sh b/scripts/commitcheck.sh new file mode 100755 index 000000000..723109112 --- /dev/null +++ b/scripts/commitcheck.sh @@ -0,0 +1,144 @@ +#!/bin/bash + +REF="HEAD" + +# test a url +function test_url() +{ + url="$1" + if ! curl --output /dev/null --max-time 60 \ + --silent --head --fail "$url" ; then + echo "\"$url\" is unreachable" + return 1 + fi +} + +# check for a tagged line +function check_tagged_line() +{ + regex='^\s*'"$1"':\s+\S+\s+\<\S+\>' + foundline=$(git log -n 1 "$REF" | egrep -m 1 "$regex") + if [ -z "$foundline" ]; then + echo "error: missing \"$1\"" + return 1 + fi + + return 0 +} + +# check for a tagged line and check that the link is valid +function check_tagged_line_with_url () +{ + regex='^\s*'"$1"':\s+\K(\S+)' + foundline=$(git log -n 1 "$REF" | grep -Po "$regex") + if [ -z "$foundline" ]; then + echo "error: missing \"$1\"" + return 1 + fi + + if ! test_url "$foundline"; then + return 1 + fi + + return 0 +} + +# check commit message for a normal commit +function new_change_commit() +{ + error=0 + + # subject is not longer than 50 characters + long_subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '.{51}') + if [ -n "$long_subject" ]; then + echo "error: commit subject over 50 characters" + error=1 + fi + + # need a signed off by + if ! check_tagged_line "Signed-off-by" ; then + error=1 + fi + + # ensure that no lines in the body of the commit are over 72 characters + body=$(git log -n 1 --pretty=%b "$REF" | egrep -m 1 '.{73}') + if [ -n "$body" ]; then + echo "error: commit message body contains line over 72 characters" + error=1 + fi + + return $error +} + +function is_openzfs_port() +{ + # subject starts with OpenZFS means it's an openzfs port + subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '^OpenZFS') + if [ -n "$subject" ]; then + return 0 + fi + + return 1 +} + +function openzfs_port_commit() +{ + # subject starts with OpenZFS dddd + subject=$(git log -n 1 --pretty=%s "$REF" | egrep -m 1 '^OpenZFS [[:digit:]]+ - ') + if [ -z "$subject" ]; then + echo "OpenZFS patch ports must have a summary that starts with \"OpenZFS dddd - \"" + error=1 + fi + + # need an authored by line + if ! check_tagged_line "Authored by" ; then + error=1 + fi + + # need a reviewed by line + if ! check_tagged_line "Reviewed by" ; then + error=1 + fi + + # need a approved by line + if ! check_tagged_line "Approved by" ; then + error=1 + fi + + # need ported by line + if ! check_tagged_line "Ported-by" ; then + error=1 + fi + + # need a url to openzfs commit and it should be valid + if ! check_tagged_line_with_url "OpenZFS-commit" ; then + error=1 + fi + + # need a url to illumos issue and it should be valid + if ! check_tagged_line_with_url "OpenZFS-issue" ; then + error=1 + fi + + return $error +} + +if [ -n "$1" ]; then + REF="$1" +fi + +# if openzfs port, test against that +if is_openzfs_port; then + if ! openzfs_port_commit ; then + exit 1 + else + exit 0 + fi +fi + +# have a normal commit +if ! new_change_commit ; then + exit 1 +fi + +exit 0 |