On Sun, Feb 04, 2018 at 10:03:18AM +0800, Chen Jingpiao wrote:
> Add the commit.signOff configuration variable to use the -s or --signoff
> option of git commit by default.
>
> Signed-off-by: Chen Jingpiao <chenjingpiao@xxxxxxxxx>
> ---
>
> Though we can configure signoff using format.signOff variable. Someone like to
> add Signed-off-by line by the committer.
This commentary explains why this feature is desirable, therefore it
would be a good idea to include this in the commit message itself.
> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> @@ -505,6 +505,75 @@ Myfooter: x" &&
> +test_expect_success "commit.signoff=true and --signoff omitted" '
> + echo 7 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'
The bodies of these test are quite noisy, doing a lot of work that isn't
really necessary, which makes it difficult to figure out what is really
being tested. Other tests in this script already check that the commit
message is properly formatted when Signed-off-by: is inserted so you
don't need to repeat all that boilerplate here.
Instead, you are interested only in whether or not Signed-off-by: has
been added to the message. For that purpose, you can use a simple 'grep'
expression.
The amount of copy/paste code in these six tests is also unfortunate.
Rather than merely repeating the same code over and over, you could
instead parameterize the test. For instance, you could run all six tests
via a simple for-loop:
--- >8 ---
for cfg in true false
do
for opt in '' --signoff --no-signoff
do
case "$opt:$cfg" in
--signoff:*|:true) expect= ;;
--no-signoff:*|:false) expect=! ;;
esac
test_expect_success "commit.signoff=$cfg & ${opt:---signoff omitted}" '
git -c commit.signoff=$cfg commit --allow-empty -m x $opt &&
eval "$expect git log -1 --format=%B | grep ^Signed-off-by:"
'
done
done
--- >8 ---
A final consideration is that tests run slowly on Windows, and although
it's nice to be thorough by testing all six combinations, you can
probably exercise the new code sufficiently by instead testing just two
combinations. For instance, instead of all six combinations, test just
these two:
--- >8 ---
test_expect_success 'commit.signoff=true & --signoff omitted' '
git -c commit.signoff=true commit --allow-empty -m x &&
git log -1 --format=%B | grep ^Signed-off-by:
'
test_expect_success 'commit.signoff=true & --no-signoff' '
git -c commit.signoff=true commit --allow-empty -m x --no-signoff &&
! git log -1 --format=%B | grep ^Signed-off-by:
'
--- >8 ---
> +test_expect_success "commit.signoff=true and --signoff" '
> + echo 8 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit --signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=true and --no-signoff" '
> + echo 9 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit --no-signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff omitted" '
> + echo 10 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff" '
> + echo 11 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit --signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --no-signoff" '
> + echo 12 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit --no-signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'
