From: Jeff King <peff@peff.net>
To: Kyle Meyer <kyle@kyleam.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs
Date: Fri, 17 Feb 2017 03:22:54 -0500 [thread overview]
Message-ID: <20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net> (raw)
In-Reply-To: <20170217035800.13214-3-kyle@kyleam.com>
On Thu, Feb 16, 2017 at 10:57:59PM -0500, Kyle Meyer wrote:
> Now that delete_refs() accepts a reflog message, pass the
> user-provided message to delete_refs() rather than silently dropping
> it. The doesn't matter for the deleted ref's log because the log is
> deleted along with the ref, but this entry will show up in HEAD's
> reflog when deleting a checked out branch.
Sounds good.
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index a41f9adf1..f642acc22 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
> */
> return delete_ref(refname,
> (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
> - flags, NULL);
> + flags, msg);
This looks obviously correct.
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index b0ffc0b57..65918d984 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -85,6 +85,15 @@ test_expect_success "delete $m (by HEAD)" '
> '
> rm -f .git/$m
>
> +test_expect_success "deleting current branch adds message to HEAD's log" '
> + git update-ref $m $A &&
> + git symbolic-ref HEAD $m &&
> + git update-ref -mdelmsg -d $m &&
> + ! test -f .git/$m &&
> + grep "delmsg$" .git/logs/HEAD >/dev/null
> +'
> +rm -f .git/$m
I think covering this with a test is good.
I don't know if it's also worth testing that deleting via HEAD also
writes the reflog. I.e.,:
git update-ref -m delete-by-head -d HEAD
Some of the style here is a bit out-dated, but I think you are just
matching the surrounding tests. So that's OK by me (though a patch to
modernize the whole thing would be welcome, too).
For reference, the two things I notice are:
- we prefer test_path_is_missing to "! test -f" these days.
- we don't redirect the output of grep (it's handled already in
non-verbose mode, and in verbose mode we try to be...verbose).
-Peff
next prev parent reply other threads:[~2017-02-17 8:22 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-16 23:17 HEAD's reflog entry for a renamed branch Kyle Meyer
2017-01-26 21:12 ` Jeff King
2017-01-26 21:30 ` Junio C Hamano
2017-01-26 21:53 ` Jeff King
2017-02-17 3:57 ` [PATCH 0/3] delete_ref(): support reflog messages Kyle Meyer
2017-02-17 3:57 ` [PATCH 1/3] delete_refs(): accept a reflog message argument Kyle Meyer
2017-02-17 8:12 ` Jeff King
2017-02-17 17:05 ` Junio C Hamano
2017-02-17 23:35 ` Kyle Meyer
2017-02-17 3:57 ` [PATCH 2/3] update-ref: pass reflog message argument to delete_refs Kyle Meyer
2017-02-17 8:22 ` Jeff King [this message]
2017-02-17 17:11 ` Junio C Hamano
2017-02-17 23:40 ` Kyle Meyer
2017-02-17 23:41 ` Jeff King
2017-02-17 3:58 ` [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log Kyle Meyer
2017-02-17 8:31 ` Jeff King
2017-02-17 17:50 ` Junio C Hamano
2017-02-17 19:43 ` Jeff King
2017-02-17 19:55 ` Jeff King
2017-02-17 22:18 ` Junio C Hamano
2017-02-17 23:42 ` Kyle Meyer
2017-02-17 23:41 ` Kyle Meyer
2017-02-17 23:53 ` Junio C Hamano
2017-02-17 23:53 ` Jeff King
2017-02-17 8:17 ` [PATCH 0/3] delete_ref(): support reflog messages Jeff King
2017-02-21 1:10 ` [PATCH v2 0/4] delete_ref: " Kyle Meyer
2017-02-21 1:10 ` [PATCH v2 1/4] delete_ref: accept a reflog message argument Kyle Meyer
2017-02-23 9:33 ` Duy Nguyen
2017-02-23 22:29 ` Junio C Hamano
2017-02-25 3:05 ` Kyle Meyer
2017-02-21 1:10 ` [PATCH v2 2/4] update-ref: pass reflog message to delete_ref() Kyle Meyer
2017-02-21 1:10 ` [PATCH v2 3/4] rename_ref: replace empty message in HEAD's log Kyle Meyer
2017-02-21 1:10 ` [PATCH v2 4/4] branch: record creation of renamed branch " Kyle Meyer
2017-02-21 7:12 ` [PATCH v2 0/4] delete_ref: support reflog messages Jeff King
2017-02-21 7:18 ` Junio C Hamano
2017-02-21 16:45 ` Kyle Meyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kyle@kyleam.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).