Skip to content

Commit

Permalink
Commit message format in contributing guidelines
Browse files Browse the repository at this point in the history
Add the need to have a commit message with a specific
format to the contributing guidelines. Provide a script
to help enforce commit message style.

Requires-builders: style
Signed-off-by: Giuseppe Di Natale <[email protected]>
  • Loading branch information
dinatale2 committed Mar 29, 2017
1 parent 12aec7d commit d39ae2c
Show file tree
Hide file tree
Showing 4 changed files with 215 additions and 1 deletion.
65 changes: 65 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -150,3 +155,63 @@ 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 70 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`.
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
```
1 change: 1 addition & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
7 changes: 6 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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.*' \
Expand Down
143 changes: 143 additions & 0 deletions scripts/commitcheck.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
#!/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()
{
RC=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"
RC=1
fi

# need a signed off by
if ! check_tagged_line "Signed-off-by" ; then
RC=1
fi

# ensure that no lines in the body of the commit are over 70 characters
body=$(git log -n 1 --pretty=%b "$REF" | egrep -m 1 '.{71}')
if [ -n "$body" ]; then
echo "error: commit message body contains line over 70 characters"
RC=1
fi

return $RC
}

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 - \""
RC=1
fi

# need an authored by line
if ! check_tagged_line "Authored by" ; then
RC=1
fi

# need a reviewed by line
if ! check_tagged_line "Reviewed by" ; then
RC=1
fi

# need a approved by line
if ! check_tagged_line "Approved by" ; then
RC=1
fi

# need ported by line
if ! check_tagged_line "Ported-by" ; then
RC=1
fi

# need a url to openzfs commit and it should be valid
if ! check_tagged_line_with_url "OpenZFS-commit" ; then
RC=1
fi

# need a url to illumos issue and it should be valid
if ! check_tagged_line_with_url "OpenZFS-issue" ; then
RC=1
fi

return $RC
}

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

0 comments on commit d39ae2c

Please sign in to comment.