cornelius.w...@tngtech.com writes:

> From: Cornelius Weig <cornelius.w...@tngtech.com>
>
> When tags are created with `--create-reflog` or with the option
> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
> So far, the description of reflog entries for tags was empty, making the
> reflog hard to understand. For example:
> 6e3a7b3 refs/tags/test@{0}:
>
> Now, a reflog message is generated when creating a tag, following the
> pattern "tag: tagging <short-sha1> (<description>)". If
> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
> (<description>)" instead. If the tag references a commit object, the
> description is set to the subject line of the commit, followed by its
> commit date. For example:
> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
>
> If the tag points to a tree/blob/tag objects, the following static
> strings are taken as description:
>
>  - "tree object"
>  - "blob object"
>  - "other tag object"
>
> Signed-off-by: Cornelius Weig <cornelius.w...@tngtech.com>
> Reviewed-by: Junio C Hamano <gits...@pobox.com>

This last line is inappropriate, as I didn't review _THIS_ version,
which is different from the previous one, and I haven't checked if
the way the comments on the previous review were addressed in this
version is agreeable.

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 072e6c6..894959f 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD 
> should succeed' '
>       test_must_fail git reflog exists refs/tags/mytag
>  '
>  
> +git log -1 > expected \
> +     --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F

We do not want to do this kind of thing outside the
test_expect_success immediately below, unless there is a good
reason, and in this case I do not see any.

Also write redirection operator and redirection target pathname
without SP in between.

>  test_expect_success 'creating a tag with --create-reflog should create 
> reflog' '
>       test_when_finished "git tag -d tag_with_reflog" &&
>       git tag --create-reflog tag_with_reflog &&
> -     git reflog exists refs/tags/tag_with_reflog
> +     git reflog exists refs/tags/tag_with_reflog &&
> +     sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog > actual &&
> +     test_cmp expected actual
> +'

In other words, something like:

test_expect_success 'creating a tag with --create-reflog should create reflog' '
        git log -1 \
                --format="format:tag: tagging %h (%s, %cd)%n" \
                --date=format:%Y-%m-%d >expected &&
        test_when_finished "git tag -d tag_with_reflog" &&
        git tag --create-reflog tag_with_reflog &&
        git reflog exists refs/tags/tag_with_reflog &&
        sed -e "s/^.*   //" .git/logs/refs/tags/tag_with_reflog >actual &&
        test_cmp expected actual
'       

Even though %F may be shorter, spelling it out makes what we expect
more explicit, and what is what I did in the above example.

Thanks.

Reply via email to