Re: [PATCH 2/2] builtin-remote: make rm operation safer in mirrored repository

[Jay Soffian <jaysoffian@gmail.com>]

On Wed, Feb 4, 2009 at 10:42 AM, Jeff King <peff@peff.net> wrote:
>> +     /* don't delete non-remote branches */
>> +     if (prefixcmp(refname, "refs/remotes")) {
>> +             if (!prefixcmp(refname, "refs/heads/"))
>> +                     string_list_append(abbrev_branch(refname),
>> +                                        branches->skipped);
>> +             return 0;
>> +     }
>
> Why does this version introduce the "only skip refs/heads/" check?
> Shouldn't we also protect other random refs (or if not, shouldn't the
> commit message explain why not)?

Note that we do protect refs, but we only emit messages about those
refs which are obviously branches. Frankly, I wasn't sure what other
kinds of refs there might be, so wasn't sure what an appropriate
message is for anything other than those under refs/heads.

In particular though, I noticed that w/o this check, I was emitting an
incorrect message about anything under refs/tags. I thought about
saying "and you can clean up these ignored tags like so", but that is
likely to emit a huge number of messages, so I thought it best just to
silently ignore non-remote non-branch refs. Perhaps I should better
explain that in a code comment.

Alternately, it could do something like:

Note: A non-remote branch was not removed; to delete it use:
   git branch -d ...

Note: Tags were not removed, to delete them use:
   git tag -d ...

Note: Some refs were ignored:
   refs/whoknows/whatthisis
   refs/whoknows/whatthiscouldbe

But that's getting a little insane me thinks.

j.