On Thu, Feb 16, 2017 at 10:58:00PM -0500, Kyle Meyer wrote:
> When the current branch is renamed, the deletion of the old ref is
> recorded in HEAD's log with an empty message. Now that delete_refs()
> accepts a reflog message, provide a more descriptive message. This
> message will not be included in the reflog of the renamed branch, but
> its log already covers the renaming event with a message of "Branch:
> renamed ...".
Right, makes sense. The code overall looks fine, though I have one
nit:
> @@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,
> return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
>
> - if (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {
> + strbuf_addf(&logmsg_del, "Deleted %s", oldrefname);
We've been anything but consistent with our reflog messages in the past,
but I think the general guideline for new ones is to use "command:
action". Of course we don't _know_ the action here, because we're deep
in rename_ref().
Should we have the caller pass it in for us, as we do with delete_ref()
and update_ref()?
I see we actually already have a "logmsg" parameter. It already says
"Branch: renamed %s to %s". Could we just reuse that? I know that this
step is technically just the deletion, but I think it more accurately
describes the whole operation that the deletion is part of.
-Peff