From 222ff6a1824da28d563e36a2b1a91abfc7fdae82 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 28 May 2024 10:57:46 +0700 Subject: [PATCH 1/2] change(common): add extra commit message hints These changes come out of discussion in the team; we want to make use of git trailers and improve the quality of our commit messages. Starting by adding more hints to the commit suggestions. Includes some minor cleanup of the hooks, and hopefully also improves tests so that they don't interfere with interactions with non-Keyman repos. Note: the scopes list may need maintenance as the list still includes sub-scopes and 'oem'. --- resources/git-hooks/commit-msg | 26 +++++++++---- resources/git-hooks/pre-commit | 22 +++++++++++ resources/git-hooks/prepare-commit-msg | 54 +++++++++++++++++--------- 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/resources/git-hooks/commit-msg b/resources/git-hooks/commit-msg index 139d7cdaf98..76c7d93643f 100755 --- a/resources/git-hooks/commit-msg +++ b/resources/git-hooks/commit-msg @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # # This commit-msg hook validates commit messages @@ -49,15 +49,18 @@ fi # Test the repository upstream/origin: ignore if not keymanapp/keyman # -if ! (git remote -v | grep -E "origin|upstream" | grep -q "keymanapp/"); then +GIT_ORIGIN="$(git remote get-url origin)" + +if [[ ! "$GIT_ORIGIN" =~ github\.com/keymanapp ]]; then # Not a Keyman repository. We have no opinion. - # echo "Not a Keyman repository. We don't care" + # echo "DEBUG: Not a Keyman repository. We don't care" exit 0 fi -if (git remote -v | grep -E "origin|upstream" | grep -Eq "keyboards|lexical-models"); then + +if [[ "$GIT_ORIGIN" =~ keyboards|lexical-models ]]; then # We don't enforce commit messages on keyboards or lexical-models repositories - # echo "Keyboards or Lexical-Models. We still don't care." + # echo "DEBUG: Keyboards or Lexical-Models. We still don't care." exit 0 fi @@ -106,11 +109,18 @@ function print_error() { echo -e "Valid scopes: ${t_grn}${scopes[@]}${t_end}" echo -e "Max length (first line): ${t_grn}$max_length${t_end}" echo -e "Min length (first line): ${t_grn}$min_length${t_end}" - echo -e "Optionally, append ${t_grn}Fixes #1234${t_end}\n" - echo -e "${t_cyn}Example:${t_end} fix(windows): Re-attaches the widget plug which had fallen out. Fixes #1111" + echo -e "If possible, append git trailers:" + echo -e " * ${t_grn}Fixes: #1234${t_end}" + echo -e " * ${t_grn}Fixes: KEYMAN-MODULE-XYZ${t_end}" + echo -e " * ${t_grn}Cherry-pick-of: #2468${t_end}" + echo -e " * ${t_grn}Co-authored-by: Firstname Lastname ${t_end}" + echo -e "${t_cyn}Example:${t_end} fix(windows): Re-attach the widget plug which had fallen out" echo -e "${t_cyn}Reference${t_end}: https://github.com/keymanapp/keyman/wiki/Pull-Request-and-Commit-workflow-notes" echo -e "" - echo -e "Tips: Don't include a period at the end of the first line; best to put 'Fixes #1111' on a line of its own." + echo -e "Tips: " + echo -e " * Don't include a period at the end of the title" + echo -e " * Always include a blank line before trailers" + echo -e " * Use imperative, present tense ('attach' not 'attaches', 'attached' etc)" echo -e "" echo -e "This script: $0" } diff --git a/resources/git-hooks/pre-commit b/resources/git-hooks/pre-commit index 3fd01c7d02a..db7ba3cf30c 100755 --- a/resources/git-hooks/pre-commit +++ b/resources/git-hooks/pre-commit @@ -1,5 +1,17 @@ #!/usr/bin/env bash +# +# for github.com/keymanapp repositories only! +# + +GIT_ORIGIN="$(git remote get-url origin)" + +if [[ ! "$GIT_ORIGIN" =~ github\.com/keymanapp ]]; then + # Not a Keyman repository. We have no opinion. + echo "DEBUG: Not a Keyman repository. We don't care" + exit 0 +fi + # # We want to make sure that .sh files are executable; however, .inc.sh need not # be as they are always source-included in scripts. @@ -16,4 +28,14 @@ if [ ! -z "$SH_NON_EXECUTABLE" ]; then exit 1 fi +# +# Prevent accidental commits to master, beta, stable-x.y, or staging (for websites) +# +branch="$(git rev-parse --abbrev-ref HEAD)" + +if [[ "$branch" =~ ^(master|beta|stable-\d+\.\d+|staging)$ ]]; then + echo "ERROR: You cannot commit to $branch for this repo. Create a branch and open a pull request\n" + exit 2 +fi + exit 0 diff --git a/resources/git-hooks/prepare-commit-msg b/resources/git-hooks/prepare-commit-msg index f6d3db9ca65..f4f6b1d7cc0 100755 --- a/resources/git-hooks/prepare-commit-msg +++ b/resources/git-hooks/prepare-commit-msg @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # # This prepare-commit-msg hook prepares commit messages to ensure @@ -13,9 +13,9 @@ # Reference: https://github.com/keymanapp/keyman/wiki/Pull-Request-and-Commit-workflow-notes # -COMMIT_MSG_FILE=$1 -COMMIT_SOURCE=$2 -SHA1=$3 +COMMIT_MSG_FILE="$1" +COMMIT_SOURCE="$2" +SHA1="$3" # Get the directory where this script lives, i.e. resources/git-hooks HOOK_DIRECTORY="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" @@ -65,17 +65,17 @@ function prepend_scope() { # COMMIT_MSG_FILE and $COMMIT_SOURCE are already available. # Step 1: if the branch follows our preferred branch conventions, we can use that! - branch=$(git rev-parse --abbrev-ref HEAD) + branch="$(git rev-parse --abbrev-ref HEAD)" build_regex # Perform the actual regex check. if [[ $branch =~ $regexp ]]; then - TYPE=${BASH_REMATCH[1]} - SCOPE=${BASH_REMATCH[2]} - # 3 - cherry-pick (if it exists) + TYPE="${BASH_REMATCH[1]}" + SCOPE="${BASH_REMATCH[2]}" + CHERRYPICK="${BASH_REMATCH[3]}" # 4 - just ISSUE, but with an appended '-'. Ashame BASH doesn't support non-capture groups. - ISSUE=${BASH_REMATCH[5]} - NAME=${BASH_REMATCH[6]} + ISSUE="${BASH_REMATCH[5]}" + NAME="${BASH_REMATCH[6]}" EXTRA_WHITESPACE="\n" @@ -86,23 +86,41 @@ function prepend_scope() { EXTRA_WHITESPACE= fi + if [ -z "$SCOPE" ]; then + prefix="$TYPE: $EXTRA_WHITESPACE" + else + prefix="$TYPE($SCOPE): $EXTRA_WHITESPACE" + fi + # Now that we have the components finalized, we can proceed. if [ -n "${ISSUE// }" ]; then # Refer to https://unix.stackexchange.com/a/146945 for explanation. # This technically does pass the conventional commits test - it doesn't -ensure- that the # "Fixes #____" section doesn't count toward the core message. - postfix="\n" - postfix="${postfix}# Keyman Conventional Commit suggestions:\n" - postfix="${postfix}# - Consider appending the text \". Fixes #$ISSUE\" to your commit message." - else - postfix="" + prefix="${prefix}\n" + prefix="${prefix}Fixes: #$ISSUE\n" fi - if [ -z "$SCOPE" ]; then - prefix="$TYPE: $EXTRA_WHITESPACE" + if [ ! -z "$CHERRYPICK" ]; then + postfix_cherrypick="\n# - Add a cherry pick trailer:\n# Cherry-pick-of: #_ID_" else - prefix="$TYPE($SCOPE): $EXTRA_WHITESPACE" + postfix_cherrypick= fi + postfix=$( + cat < +# - Use imperative, present tense ('attach' not 'attaches', 'attached' etc) +# - Don't include a period at the end of the title +# - Always include a blank line before trailers +# - More: https://github.com/keymanapp/keyman/wiki/Pull-Request-and-Commit-workflow-notes +EOF + ) + # Reuse any existing message text, wrapping it in our conventional commit formatting before # presenting it to the user for any final edits. echo -e "$prefix$(cat $COMMIT_MSG_FILE)$postfix" > $COMMIT_MSG_FILE From b3385e4d692051cb317df89453a46b04a5c50ca5 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 30 May 2024 12:30:33 +1000 Subject: [PATCH 2/2] chore: Apply suggestions from code review Co-authored-by: Eberhard Beilharz --- resources/git-hooks/commit-msg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/git-hooks/commit-msg b/resources/git-hooks/commit-msg index 76c7d93643f..427ad96b58c 100755 --- a/resources/git-hooks/commit-msg +++ b/resources/git-hooks/commit-msg @@ -120,7 +120,7 @@ function print_error() { echo -e "Tips: " echo -e " * Don't include a period at the end of the title" echo -e " * Always include a blank line before trailers" - echo -e " * Use imperative, present tense ('attach' not 'attaches', 'attached' etc)" + echo -e " * Use imperative, present tense ('attach' instead of 'attaches', 'attached' etc)" echo -e "" echo -e "This script: $0" }