diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0d7122a739..8df6513e5c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -149,6 +149,10 @@ It is possible to check if your code follows these conventions: simple as it is. ``` +* The commit message is automatically checked by the static tests for keywords + that prevent a merge, for example "fixup" or "DONOTMERGE". The full list + of these keywords is stored in `dist/tools/pr_check/no_merge_keywords`. + ### Pull Requests [pull requests]: #pull-requests @@ -304,6 +308,9 @@ $ git commit --fixup ### Squash commits after review +*** Note: If the static tests warn you about a no-merge keyword, please look +at [our commit conventions][commit-conventions]. + Squashing a commit is done using the rebase subcommand of git in interactive mode: @@ -323,7 +330,7 @@ phase, squashing commits can be performed in a single command: $ git rebase -i --autosquash ``` -**Watch out: Don't squash your commit until a maintainer asks you to do it.** +***Watch out: Don't squash your commit until a maintainer asks you to do it.*** Otherwise the history of review changes is lost and for large PRs, it makes it difficult for the reviewer to follow them. It might also happen that diff --git a/dist/tools/commit-msg/check.sh b/dist/tools/commit-msg/check.sh index ba4cdb1489..8e4220e17a 100755 --- a/dist/tools/commit-msg/check.sh +++ b/dist/tools/commit-msg/check.sh @@ -6,6 +6,7 @@ # General Public License v2.1. See the file LICENSE in the top level # directory for more details. +# shellcheck source=/dev/null . "$(dirname "$0")/../ci/github_annotate.sh" github_annotate_setup @@ -13,7 +14,7 @@ github_annotate_setup MSG_MAX_LENGTH=50 MSG_STRETCH_LENGTH=72 -if tput colors 2> /dev/null > /dev/null && [ $(tput colors) -ge 8 ]; then +if tput colors 2> /dev/null > /dev/null && [ "$(tput colors)" -ge 8 ]; then CERROR1="\033[1;31m" CWARN1="\033[1;33m" CERROR2="\033[0;31m" @@ -31,14 +32,14 @@ if [ -n "${BASH_VERSION}" ]; then # properly in _escape ECHO_ESC='echo -e' else - ECHO='echo' + ECHO_ESC='echo' fi # If no branch but an option is given, unset BRANCH. # Otherwise, consume this parameter. BRANCH="${1}" if echo "${BRANCH}" | grep -q '^-'; then - if [ $(git rev-parse --abbrev-ref HEAD) != "master" ]; then + if [ "$(git rev-parse --abbrev-ref HEAD)" != "master" ]; then BRANCH="master" else BRANCH="" @@ -55,13 +56,15 @@ if [ -z "${BRANCH}" ]; then fi ERROR="$(git log \ - --no-merges --pretty=format:'%s' $(git merge-base ${BRANCH} HEAD)..HEAD | \ - while read msg; do - msg_length=$(echo "${msg}" | awk '{print length($0)}') + --no-merges --pretty=format:'%s' "$(git merge-base "${BRANCH}" HEAD)"..HEAD | \ + while read -r msg; do + ERROR=0 + MSG="" - if [ ${msg_length} -gt ${MSG_MAX_LENGTH} ]; then - ERROR=0 - if [ ${msg_length} -gt ${MSG_STRETCH_LENGTH} ]; then + # Check if the message is too long. + msg_length="$(echo "${msg}" | awk '{print length($0)}')" + if [ "${msg_length}" -gt "${MSG_MAX_LENGTH}" ]; then + if [ "${msg_length}" -gt "${MSG_STRETCH_LENGTH}" ]; then MSG="Commit message is longer than ${MSG_STRETCH_LENGTH} characters:" ERROR=1 echo "error" @@ -69,6 +72,23 @@ ERROR="$(git log \ MSG="Commit message is longer than ${MSG_MAX_LENGTH}" MSG="${MSG} (but < ${MSG_STRETCH_LENGTH}) characters:" fi + fi + + # Check if the message has a colon and if the first colon is followed + # by a space. + if echo "$msg" | grep -q ":"; then + if ! echo "$msg" | grep -Eq '^[^:]*: '; then + MSG="The colon after the area designation is not followed by" + MSG="${MSG} a space." + fi + else + MSG="The commit message is missing a colon after the area" + MSG="${MSG} designation." + ERROR=1 + echo "error" + fi + + if [ -n "${MSG}" ]; then if github_annotate_is_on; then if [ ${ERROR} -eq 1 ]; then github_annotate_error_no_file "${MSG} \"${msg}\"" diff --git a/dist/tools/pr_check/check.sh b/dist/tools/pr_check/check.sh index 6796d754b6..d720b7e939 100755 --- a/dist/tools/pr_check/check.sh +++ b/dist/tools/pr_check/check.sh @@ -7,18 +7,26 @@ # directory for more details. # +# shellcheck source=/dev/null . "$(dirname "$0")/../ci/github_annotate.sh" -: "${RIOTBASE:=$(cd $(dirname $0)/../../../; pwd)}" -cd $RIOTBASE +: "${RIOTBASE:="$(cd "$(dirname "$0")"/../../../ || exit; pwd)"}" +cd "$RIOTBASE" || exit : "${RIOTTOOLS:=${RIOTBASE}/dist/tools}" EXIT_CODE=0 +# Keywords that should trigger the commit message check and prevent an accidental +# merge of something not meant to be merged. +# The pretty-print format of the commit messages is always the following: +# `a5e4f038b8 commit message` +# This has to be reflected in the RegEx matching pattern. +NOMERGE_KEYWORD_FILE="$(dirname "$0")/no_merge_keywords" + github_annotate_setup -if tput colors &> /dev/null && [ $(tput colors) -ge 8 ]; then +if tput colors &> /dev/null && [ "$(tput colors)" -ge 8 ]; then CERROR="\e[1;31m" CRESET="\e[0m" else @@ -32,30 +40,28 @@ else RIOT_MASTER="master" fi -keyword_filter() { - grep -i \ - -e "^ [0-9a-f]\+ .\{0,2\}SQUASH" \ - -e "^ [0-9a-f]\+ .\{0,2\}FIX" \ - -e "^ [0-9a-f]\+ .\{0,2\}REMOVE *ME" \ - -e "^ [0-9a-f]\+ .\{0,2\}Update" -} - -SQUASH_COMMITS="$(git log $(git merge-base HEAD "${RIOT_MASTER}")...HEAD --pretty=format:" %h %s" | \ - keyword_filter)" +SQUASH_COMMITS="$(git log "$(git merge-base HEAD "${RIOT_MASTER}")"...HEAD --pretty=format:"%h %s" | \ + grep -i -f "${NOMERGE_KEYWORD_FILE}")" if [ -n "${SQUASH_COMMITS}" ]; then if github_annotate_is_on; then - echo "${SQUASH_COMMITS}" | while read commit; do - ANNOTATION="Commit needs to be squashed: \"${commit}\"" - ANNOTATION="${ANNOTATION}\n\nPLEASE ONLY SQUASH WHEN ASKED BY A " + ANNOTATION="" + while read -r commit; do + ANNOTATION="${ANNOTATION}Commit needs to be squashed or contains a no-merge keyword: \"${commit}\"\n" + done < <(echo "${SQUASH_COMMITS}") + + if [ -n "${ANNOTATION}" ]; then + ANNOTATION="${ANNOTATION}\nPLEASE ONLY SQUASH WHEN ASKED BY A " ANNOTATION="${ANNOTATION}MAINTAINER!" ANNOTATION="${ANNOTATION}\nSee: " - ANNOTATION="${ANNOTATION}https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#squash-commits-after-review" + ANNOTATION="${ANNOTATION}https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#squash-commits-after-review\n" github_annotate_error_no_file "${ANNOTATION}" - done + fi else - echo -e "${CERROR}Pull request needs squashing:${CRESET}" 1>&2 - echo -e "${SQUASH_COMMITS}" + echo -e "${CERROR}Pull request needs squashing or contains no-merge keywords:${CRESET}" 1>&2 + while IFS= read -r line; do + echo -e " ${line}" + done <<< "${SQUASH_COMMITS}" fi EXIT_CODE=1 fi diff --git a/dist/tools/pr_check/no_merge_keywords b/dist/tools/pr_check/no_merge_keywords new file mode 100644 index 0000000000..eec145fa04 --- /dev/null +++ b/dist/tools/pr_check/no_merge_keywords @@ -0,0 +1,12 @@ +[0-9a-f]\+ SQUASH +[0-9a-f]\+ FIX +[0-9a-f]\+ REMOVE *ME +[0-9a-f]\+ Update +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \ +[0-9a-f]\+ \